Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jsr: imports #957

Merged
merged 27 commits into from
Sep 18, 2024
Merged

jsr: imports #957

merged 27 commits into from
Sep 18, 2024

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Mar 2, 2024

Ref. https://jsr.io/. Fixes #956. TODO

  • Resolve jsr: imports
  • Populate the _jsr cache during resolution
  • Resolve and rewrite transitive jsr: imports (e.g., @jsr/quentinadam__assert for jsr:@quentinadam/hex)
  • Resolve and rewrite transitive npm: imports (e.g., jsr:@oak/oak depends on npm:path-to-regexp)
  • Preload transitive imports
  • Respect the dependency version range specified in package.json
  • Adopt resolve.exports for more correct conditional export resolution
  • Documentation
  • Tests

@mbostock mbostock requested a review from Fil March 2, 2024 18:40
@mbostock
Copy link
Member Author

mbostock commented Mar 2, 2024

It’s alive!

Screenshot 2024-03-02 at 10 50 56 AM

@mbostock mbostock marked this pull request as ready for review September 12, 2024 14:29
@Fil
Copy link
Contributor

Fil commented Sep 13, 2024

I tried a few of the modules listed, to find some good stuff and make tests.

@libs/markdown: import * as mod from "jsr:@libs/markdown"; crashed with Error: Missing "./slugify" specifier in "@jsr/std__text" package. Did not find how to fix this one.

@ch/tomd: import { tomd } from "jsr:@ch/tomd" displays SyntaxError: The requested module '/_npm/[email protected]/_esm.js' does not provide an export named 'Browser'

I managed to crash the preview server when I tried to import k-means-pp from jsr:k-means-pp (the correct specifier was jsr:@ppz/k-means-pp), my mistake.

(Maybe these errors can be controlled?)

Most modules I've tried worked out the the box. There is good stuff: @ppz/k-means-pp@storynode/jql@dbushell/hyperless@notwoods/spherical-geometry-js

I'm watching https://www.youtube.com/watch?v=MFCn4ce5dVc for the rationale.

@mbostock
Copy link
Member Author

Not sure if we will be able to fix any of those errors, but I can look. Do you think they are blocking?

@Fil
Copy link
Contributor

Fil commented Sep 13, 2024

No, not blocking. I'm now trying to publish a package to see what happens "end-to-end".

The whole experience seems very nice.

@mbostock
Copy link
Member Author

The slugify error seems like a bug in @libs/markdown. In @std/text 1.0.5, slugify was available but documented as unstable; in @std/text 1.0.6 it was moved to unstable_slugify. But @libs/markdown says it just depends on @std/text 1, but it needs exactly 1.0.3, 1.04, or 1.0.5 to work. It has 1.0.4 in its deno.lock file but I don’t think we have to (or should) respect lock files when resolving transitive dependencies.

@mbostock
Copy link
Member Author

The @ch/tomd error is because we’re trying to load the happy-dom package from jsDelivr, but jsDelivr refuses to serve it and replaces it with an error saying:

Failed to bundle using Rollup v2.79.1: the file imports a not supported node.js built-in module "node:perf_hooks". If you believe this to be an issue with jsDelivr, and not with the package itself, please open an issue at https://github.com/jsdelivr/jsdelivr'

I’m not entirely sure why jsDelivr’s error isn’t shown; but at any rate the error saying that it doesn’t export a symbol called Browser is accurate, because jsDelivr wasn’t able to bundle the code. I don’t think we need to do anything for this one either; this is just a package that doesn’t work in a browser environment. (Though perhaps it would be nice to detect when jsDelivr fails to bundle the code, rather than downloading a broken library… I can file a bug for that.)

@mbostock
Copy link
Member Author

I fixed the jsr:k-means-pp error, and also am now populating the in-memory JSR version resolution cache correctly. There are probably some more bugs to fix here. We need more tests.

@mbostock mbostock enabled auto-merge (squash) September 18, 2024 01:02
@mbostock mbostock merged commit 9af438a into main Sep 18, 2024
4 checks passed
@mbostock mbostock deleted the mbostock/jsr branch September 18, 2024 01:05
@mootari mootari mentioned this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSR imports
2 participants