-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Support exports
syntax in package.json
#187
Comments
I've been following along with discussions about this on the issue trackers for other bundlers. As far as I know the |
I think that is a prudent approach, but should at least be documented in a section about the module resolution strategy. Node 14 is advocating its use export takes precendence over main which will lead to some surprises. |
Thanks for the links. However, it looks like decisions on how this stuff is supposed to work are still settling. See for example this long Twitter thread. Big +1 from me to what Devon is advocating for. I'd like to wait until y'all figure it out first so I don't end up implementing something that doesn't follow the community conventions. |
@evanw Shouldn't it be safe to implement Node's algorithm at this point though? It has been shipped as stable for 3 major versions, which makes it pretty locked in at this point. The additional community-proposed fields can certainly be subject to change, but that may always be the case, and they're only ever going to be a layer atop the base 3 fields. |
How about using the resolve.exports package by @lukeed? |
Thanks! Yes that package follows the Node.js implementation and supports all
This makes it able to support all existing bundlers & whatever conditions they may choose to invent. Vite is already using it, wmr and yarn are WIP, and Rollup has stated that they're open to swapping too. |
In the meantime, we could probably write a plugin that used |
If you're looking at writing a plugin, you may find this recently-released feature helpful for the import vs. require distinction: #879.
Right now my main reason for not doing this right now (at least a node-compatible version) is that doing this is a breaking change. This change is on my list of potential things to bundle together in the next upcoming breaking change release. |
@evanw appreciate your great work ! You do not have to make this a breaking change though if you hide it behind a feature flag. Do you think that would add too much complexity ? |
esbuild still doesn't support exports evanw/esbuild#187 it's uncertain as yet what impact this might have on other bundlers and loaders, hence this is a BREAKING CHANGE.
esbuild still doesn't support exports evanw/esbuild#187 it's uncertain as yet what impact this might have on other bundlers and loaders, hence this is a BREAKING CHANGE.
* fix!: don't delete "main" in package.json esbuild still doesn't support exports evanw/esbuild#187 it's uncertain as yet what impact this might have on other bundlers and loaders, hence this is a BREAKING CHANGE. * fixup! fix!: don't delete "main" in package.json
This feature has been released in version 0.9.0 by the way. Please try it out and let me know if anything seems off. I tried to follow node's specification as close as I could but I don't use this feature myself so it's possible that something might not be working exactly right on the first try. |
The module landscape is a mess. Currently the UMD build is preferred by Webpack and esbuild, since the `browser` field takes precedence over the `module` field. However, conditional exports (yet another field!) now allow for a separate approach. Setting the first type to `import` results in both bundlers favouring it over the UMD build. I'm leaving Browser around as a legacy field for now. See: - webpack/webpack#4674 - evanw/esbuild#187 - https://webpack.js.org/guides/package-exports/#providing-different-versions-depending-on-target-environment - https://nodejs.org/api/packages.html#packages_exports_sugar
Hi @evanw, I think I have an unsupported case, into the project of my clients: "exports": {
"./*": "./src/*"
}, |
That already works: |
Hi @evanw, Thanks a lot for your reply & sorry for the delay, I was investigating... but even if it works within your playground... the bundling fails for my client project files. Since, as you said, you don't really use the After the download: cd client
npm i
npm run build EDIT: |
The failing import is in the
That doesn't seem like a problem with esbuild to me. |
Hum, since components-generics is a client dependency... there is no reasons to go into the components-generics to install it distincly... just like any remote dependency (like the https://www.npmjs.com hosted ones), it isn't? Moreover, it's a distinct behavior from rollup on that point. |
The problem you're describing also happens when you run that code in node: $ node src/js/index.js
node:internal/modules/esm/resolve:854
throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
^
Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@owner/front' imported from ./esbuild-attempt/components-generics/src/js/generics.js Since esbuild implements node's path resolution algorithm, this problem also happens in esbuild. So what esbuild is doing is the expected behavior. Node invented the You can see what's happening if you pass
The import path |
Oh, I see... thanks a lot and sorry for that, then 🫣 |
More modern ESM node modules are supposed to use
exports
syntax to control visibility of exports.https://nodejs.org/api/esm.html#esm_main_entry_point_export
In case this is something that could be done an issue should track any progress
The text was updated successfully, but these errors were encountered: