react-datetime icon indicating copy to clipboard operation
react-datetime copied to clipboard

Add a "browser" field so build tools can find UMD build

Open davidwallacejackson opened this issue 3 years ago • 3 comments

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

davidwallacejackson avatar Nov 26 '21 06:11 davidwallacejackson

the umd could be the main.

morlay avatar Nov 26 '21 09:11 morlay

@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.

davidwallacejackson avatar Nov 29 '21 03:11 davidwallacejackson

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.

fabianonunes avatar Dec 13 '21 13:12 fabianonunes