Skip to content
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 a "browser" field so build tools can find UMD build #813

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidwallacejackson
Copy link

Description

Right now, the package.json only indicates the presence of the CommonJS build of react-datetime (as "main"). This change exposes the UMD build via the "browser" field.

Motivation and Context

Right now, Rollup loads the CJS version of react-datetime through @rollup/plugin-commonjs. That should work just fine, but there are some edge cases in the plugin -- this one in particular seems problematic for react-datetime.

I discovered all this by trying to import react-datetime in Vite and discovering that the library worked in development (where Vite implements its own esbuild-based CJS -> ESM transformation) but not in production (where Vite uses Rollup). Here's a minimal repro.

Including ESM directly would be ideal, but webpack still lists ESM output support as "experimental" in their docs, and I'm assuming this project doesn't particularly want to migrate to another build tool. I tried this out in my local copy and it fixed the issue.

Checklist

[x] I have not included any built dist files (us maintainers do that prior to a new release)
[ ] I have added tests covering my changes
[x] All new and existing tests pass
[ ] My changes required the documentation to be updated
  [ ] I have updated the documentation accordingly
  [ ] I have updated the TypeScript 1.8 type definitions accordingly
  [ ] I have updated the TypeScript 2.0+ type definitions accordingly

@morlay
Copy link

morlay commented Nov 26, 2021

the umd could be the main.

@davidwallacejackson
Copy link
Author

@morlay it could -- I'm not sure of the merits/appropriateness of using umd as the main entrypoint? I did this with "browser" because I found other projects doing it, and it fixed the issue I was having in Vite, but I have no strong reason to prefer this approach over the other.

@fabianonunes
Copy link

fabianonunes commented Dec 13, 2021

It may not be necessary to export the library in another format, although it is the best option. I think passing libraryExport="default" to webpack config is enough:

// config/webpack.config.build.js
const cjsConfig = {
	...baseConfig,
	output: {
		path: outputPath,
		library: 'Datetime',
		libraryTarget: 'commonjs2',
		filename: 'react-datetime.cjs.js',
		auxiliaryComment: 'React datetime',
+               libraryExport: "default"
	}
};

This isn't a Vite issue but a @rollup/plugin-commonjs bug introduced by the refactoring done in rollup/plugins#537. To confirm, you can downgrade @rollup/plugin-commonjs to version 15.0.0.

NiBeliaev added a commit to NiBeliaev/react-datetime that referenced this pull request Jul 26, 2022
Add a browser field to fix build issue
arqex#813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants