-
-
Notifications
You must be signed in to change notification settings - Fork 159
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
Add ESM build script #154
Add ESM build script #154
Conversation
Awesome, thank you! I know there has been past discussions, so not sure if you have seen all of it, but a couple things for this PR i noticed missing, if you can think of how we can solve:
In addition, one of the pervious issues was that why does this build step need to exist at all? Node.js provides a way to have both cjs ans esm without any build steps needed. The last time it was due to Vite but the user decided it was a Vite bug and not to fix it here. Does that apply still, or can we simply follow the Node.js docs like https://nodejs.org/api/packages.html#approach-1-use-an-es-module-wrapper ? Myself and I'm sure anyone else consuming esm is really thankful. |
Thanks for the feedback, those changes make sense. As for why this is needed, my use case was Rollup which by default doesn't support common js (even via the wrapper method suggested in the Node docs). There is a plugin to enable it which I currently use, but if I can reduce my dependencies and config complexities, I would rather do that. Plus I believe that given ESM modules have been the standard for a while we will start to see more build tools and other environments that do not support CJS, so being prepared for that possibility would be beneficial. |
@dougwilson It should be simple to run all the tests twice (once for each export type) by wrapping them in a function or loop (any preference?) |
FYI I am waiting for confirmation that this approach sounds good before I go ahead and do the work to implement this |
@dougwilson I think your confirmation is needed. I only have this dep that that don't have ESM support. @PaulKiddle thanks, hope you still helping this. |
Hi @PaulKiddle, thanks for the PR! I'm trying to tidy up some older PRs in this repo and I'm going to close this one as we won't be adding dual package publishing in the immediate future. As far as I understand, most bundlers support CommonJS. I think this will make sense to revisit if/when we go ESM only. |
Adds a script to create the ESM version of the code, fixing #152
The script is automatically run on
prepare
(i.e. when packaging for release or installing directly from git) on npm versions ≥ 4The generated file is not committed to the repository