-
Notifications
You must be signed in to change notification settings - Fork 33
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
chore(bueno): improve exports, externalize dependencies from bundles #4987
base: master
Are you sure you want to change the base?
Conversation
louis-bompart
commented
Feb 19, 2025
- Remove top-level browser export specifier: this was not up to standard; see npm doc
- Add conditional-exports to provide wide compability.
- Externalize dependencies from the bundle:
- It is expected that the final consumers of the package (i.e. those who push the bundles for the browser) do bundles.
- Bundling in dependencies may create duplicate code in final consumer bundles
- Move CDN/browser output outside packaged files (logic follow-up)
b1eb26b
to
48f0f09
Compare
Pull Request ReportPR Title✅ Title follows the conventional commit spec. Live demo linksBundle Size
|
@@ -29,6 +30,7 @@ function nodeEsm() { | |||
return build({ | |||
...base, | |||
platform: 'node', | |||
packages: 'external', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bueno has no dependencies !
"types": "./dist/definitions/index.d.ts", | ||
"import": "./dist/bueno.esm.js", | ||
"require": "./dist/bueno.js", | ||
"default": "./dist/bueno.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we put ESM as default ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it, but I think it might be breaking
a421fbb
to
73a2e9f
Compare