chore: Add an ESM submodule export to the build output #1129
Conversation
Demo
Jenkins 
demos.haus 
goulinkh
changed the title
package.json
Outdated
| "build": "rm -rf dist && yarn build-cjs && yarn build-declaration && yarn build-esm && yarn build-declaration-esm", | ||
| "build-cjs": "NODE_ENV=production BABEL_ENV=cjs babel src --out-dir dist --copy-files --extensions '.js,.jsx,.ts,.tsx'", |
There was a problem hiding this comment.
As-is, this outputs CJS into dist and esm into dist/esm. This makes it possible for CJS output files to be in the same directory if the input filestructure is wrong.
It may be a bit cleaner to also put cjs into its own folder (dist/cjs) and update the package index to dist/cjs/index.js.
I think we'd then need to update package.json main to dist/cjs/index.js and set module to dist/esm/index.js
There was a problem hiding this comment.
I understand your point, and I agree with you. Having both types separated into two different directories is much cleaner.
The reason for leaving the cjs version as it is, is mainly to avoid breaking existing projects that are using this package with absolute paths to components. I'm sure that these imports aren't needed as everything should be importable from /dist/cjs/index.js.
Here is a set of projects that have this problem, this list is probably only a subset of the total projects that have this issue: https://github.com/search?q=org%3Acanonical+canonical%2Freact-components%2Fdist&type=code
There was a problem hiding this comment.
I would still tend to have them in two separate folders. I believe having them in the same is suboptimal and can lead to a lot of headaches - and will impact future maintainability.
A solution could be to remap manually the user imports using the package.json exports field - it allows for a cleaner folder structure but is not necessarily elegant.
If we accept the two folders one in the other, I believe we should clearly document this somewhere as part of this PR. If this helps we could do that for the time being - pending the new architecture.
|
@advl Yes it works! I'm currently using it in Astro and it works as expected. My changes handle Jest, Storybook with Cypress. |
package.json
Outdated
| "build": "rm -rf dist && yarn build-cjs && yarn build-declaration && yarn build-esm && yarn build-declaration-esm", | ||
| "build-cjs": "NODE_ENV=production BABEL_ENV=cjs babel src --out-dir dist --copy-files --extensions '.js,.jsx,.ts,.tsx'", |
There was a problem hiding this comment.
I would still tend to have them in two separate folders. I believe having them in the same is suboptimal and can lead to a lot of headaches - and will impact future maintainability.
A solution could be to remap manually the user imports using the package.json exports field - it allows for a cleaner folder structure but is not necessarily elegant.
If we accept the two folders one in the other, I believe we should clearly document this somewhere as part of this PR. If this helps we could do that for the time being - pending the new architecture.
|
Demo failed to build, rebuilding to verify things work OK |
|
🎉 This PR is included in version 1.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…ical#1129)" This reverts commit e0b66f7.
Done