-
Notifications
You must be signed in to change notification settings - Fork 405
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
Publish both esm and cjs #438
Conversation
Codecov Report
@@ Coverage Diff @@
## main #438 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 26
Lines 624 624
Branches 231 231
=========================================
Hits 624 624 Continue to review full report at Codecov.
|
To be safe, maybe this could be released along with #379 in a major version bump. |
Ok. I'll do it that way. So for now I'll merge this onto the One thing I wanted to ask you about: what are the chances for surprises when this gets released? I ask because I'm a bit nervous that this cannot be tested locally? Or can it? Can I test somehow how this would work as if it was installed via npm, but without the release existing yet? Have you tested it in this way somehow? |
@gnapse I have tested in my own vite storybook app, by running I don't have a traditional jest project running in node to test the cjs build on, but the process would be the same. Perhaps you could publish an |
@gnapse do you still plan to release a beta version with this change? Is there anything else you'd like me to do before you're comfortable with it? |
@IanVS sorry for the delay. I'm going to resume work on this tomorrow. I'll decide then if I go with a alpha or beta version first. |
Thank you all for doing this! I was wondering whether there are plans to release this in an alpha version somewhere? Thank you <3 |
Hi is there any plan to release this? It's kind of a bummer that I put the work into it and there's still no way to use it. |
Hey @gnapse was this change ever included in a release? |
Can this PR also be merged to main? |
It looks like the other changes that were in |
OK, I see now this needs to be reworked a bit based on the multiple export paths now supported for vitest, etc. I'll open a new PR. Edit: #519 |
What:
This publishes both es modules and commonjs in the bundle.
Why:
Closes #437
How:
I used rollup to generate the two sets of bundles. I'm not doing any processing on either of them aside from what rollup does by default, mostly because I'm not sure what level of browser/node support this project follows. It would be pretty straightforward to add https://www.npmjs.com/package/@rollup/plugin-babel or otherwise configure the output as needed.
Checklist:
I haven't updated any documentation or tests, because I wanted to see if this was at all on the right track.