-
Notifications
You must be signed in to change notification settings - Fork 36
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
Improve package publishing #701
Comments
|
|
If you want to reduce memory usage you can still bundle your server code and drop unused code via tree shaking. I don't see why deep imports (which limit me as a library maintainer), should take care of this instead. I think importing from CDN and causing request waterfalls is also something that should be avoided for production usage (by using a bundler), for development/demos the additional loaded code won't be a dealbreaker. The dual package hazard is only introduced if people decide to only ship ESM or commonjs which basically means "I don't care about library consumers" to me. If a package has zero dependencies (or all dependencies of that package are ESM only) I agree that only shipping ESM is totally legit. But as soon your library has dependencies you should ship both commonjs and ESM in order to avoid the dual package hazard. |
There would be no need for the complexities of bundling and tree shaking if authors would publish API's in this way. Users won't be able to deep import just anything, only what you choose to export as the public API.
This is incorrect. The issue is that these packages are publishing both ESM and commonjs in seperate bundles, that will cause a dual package hazard. See here for an explanation: https://nodejs.org/api/packages.html#packages_dual_package_hazard. Essentially the same singletons should be exported from both ESM and commonjs entrypoints. If you continue bundling, just have the ESM entrypoint import and export from the bundled commonjs entrypoint. Ideally, though as there's no real reason to bundle, just publish unbundled commonjs and have the two entrypoints import and export from those files. |
@dburles Can you provide a reproduction, where this library would be affected by dual package hazard in a real-world scenario? I would love to get some more insights on this, as it seems that I cannot fully understand the situation. |
Hey @n1ru4l, this repo gives a pretty good example where the problem can arise https://github.com/GeoffreyBooth/dual-package-hazard |
@dburles Can you provide an example woth packages in this mono-repo? |
That is not always easy to determine ahead of time, which is why it's a hazard that's best avoided, since it could manifest itself in unexpected ways. Given that this project is published in a very modular way, I would guess that you might expect others to potentially publish their own graphql-live-query utility package that consumes parts of the graphql-live-query API, and because of that, users are then prone to potential issues. Anything publishing dual packages with https://github.com/kamilkisiela/bob suffers from the dual package hazard, since it creates two copies of the same code as both cjs and esm. If you wish to continue publishing in this way (which I wouldn't recommend), it would be best to maintain an awareness of the pitfalls, as explained here: https://nodejs.org/api/packages.html#approach-2-isolate-state. It's a complex thing to grasp, so I hope that I've helped explain it a little. The Node docs do explain it very well, so have a read through that if you haven't already, https://nodejs.org/api/packages.html#dual-commonjses-module-packages. |
For some additional clarity, these packages are currently published correctly if they included only the I'll quote part of the Node docs on the dual package hazard as I think it will also be useful to others reading this issue:
Dual package hazard
|
Here's a simple app (https://github.com/dburles/dph-graphql-live-query) that demonstrates how it's possible to import into memory and run two copies of the same function. The demo app is denoted as a "module" by the top level The I've also added an example of when bundling this application, both the ESM and commonjs source of As stated above: "While it is unlikely that an application or package would intentionally load both versions directly (as in this demo -David), it is common for an application to load one version while a dependency of the application loads the other version." |
Suggestions:
exports
field shouldn't stripped from the publishedpackage.json
and noindex.mjs
file is included. @n1ru4l I found that you raised this issue earlier, (though not sure why it was closed) exports map and .mjs extension kamilkisiela/bob#16The text was updated successfully, but these errors were encountered: