-
Notifications
You must be signed in to change notification settings - Fork 233
Add ESM support for browser environments #278
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
base: master
Are you sure you want to change the base?
Conversation
|
This seems to behave correctly, at least when generating an import map with "scopes": {
"./": {
"bson": "https://unpkg.com/bson@7.0.0/lib/bson.mjs",
"cbor2": "https://unpkg.com/cbor2@2.0.1/lib/index.js",
"eventemitter3": "https://unpkg.com/eventemitter3@5.0.1/dist/eventemitter3.esm.js",
"fast-png": "https://unpkg.com/fast-png@7.0.1/lib/index.js",
"uuid": "https://unpkg.com/uuid@13.0.0/dist/index.js",
"ws": "https://unpkg.com/ws@8.18.3/browser.js"
},
"https://unpkg.com/": {
"@cto.af/wtf8": "https://unpkg.com/@cto.af/wtf8@0.0.2/lib/index.js",
"iobuffer": "https://unpkg.com/iobuffer@6.0.1/lib/iobuffer.js",
"pako": "https://unpkg.com/pako@2.1.0/dist/pako.esm.mjs"
}
}(taken from the project I have been debugging this issue on) |
|
This makes bundlers use the already bundled versions. I'm not very happy with it. I think it does not work well with source maps. All I can think of is adding an export for |
|
That isn't true in my testing. Vite is still bundling |
|
You are right, the "browser" condition is under "exports" here, not top level. Anyway is adding it after "import" and "require" correct? How is it evaluated? |
|
Hmm, unsure. The Node.js docs say the rules should be ordered from most specific to least specific, so maybe this should be at the top instead. |
|
Yes, after "types". |
|
Moving it at the top (after "types") makes esbuild (and I guess other bundlers) use the dist files. |
d66a861 to
a10d558
Compare
|
Interesting that leaving it last seems to allow bundlers to continue using the unbundled code but allows |
a10d558 to
c082c0c
Compare
|
Okay, trying to outline the real problem here:
So far I've yet to find a fix for this. I might just have to keep my |
|
As written above I think a possible workaround is to export the bundles diff --git a/package.json b/package.json
index 553284b..717998b 100644
--- a/package.json
+++ b/package.json
@@ -7,6 +7,7 @@
"import": "./index.mjs",
"require": "./index.js"
},
+ "./*.js": "./dist/*.js",
"./package.json": "./package.json"
},
"main": "index.js",so that for cases like #277 it is possible to do something like this import EventEmitter from 'eventemitter3/eventemitter3.esm.js'; |
|
Yeah, I was hoping to find something nicer since my library supports both Node and the browser and I'd rather have sourcemaps work. But that may be the only solution for now. |
Fixes #277 by specifying both a CJS and ESM import path for browsers.
Supersedes #272.