Skip to content

Create ES dev bundle same as CJS#141

Closed
danielkcz wants to merge 5 commits into
jaredpalmer:masterfrom
danielkcz:es-dev-bundle
Closed

Create ES dev bundle same as CJS#141
danielkcz wants to merge 5 commits into
jaredpalmer:masterfrom
danielkcz:es-dev-bundle

Conversation

@danielkcz

Copy link
Copy Markdown
Contributor

Fixes #140

Tested it against a project that has shown the problem and works correctly there.

Btw: It seems that tests setup is wrong, no test file is executed.

@jaredpalmer

jaredpalmer commented Jun 12, 2019 via email

Copy link
Copy Markdown
Owner

@danielkcz

danielkcz commented Jun 12, 2019

Copy link
Copy Markdown
Contributor Author

@jaredpalmer Well, the assumption is that you will use index.es.js in the module field the same way you have main for CJS format.

@TrySound

Copy link
Copy Markdown
Collaborator

This way you may end up with a lot of duplicated code in your bundle.

@danielkcz

Copy link
Copy Markdown
Contributor Author

@TrySound Well yea, but how does that matter? Each bundler/environment will pick whatever they need. It's not like you will use all of them at once.

UMD is the ultimate copy and would forbid for that format to exists if I could :)

@danielkcz

Copy link
Copy Markdown
Contributor Author

Ah, finally it's green. Please squash it, I got a bit overboard with commits here 😎

@TrySound

Copy link
Copy Markdown
Collaborator

The point is to drop commonjs completely. It should not be used to import esm. It will break a lot of stuff.

@danielkcz

danielkcz commented Jun 12, 2019

Copy link
Copy Markdown
Contributor Author

@TrySound That's a bad idea. NodeJS is still not fully ESM capable, especially for people stuck at V8 or something. You cannot ask of them to transpile 3rd party modules.

@TrySound

TrySound commented Jun 12, 2019

Copy link
Copy Markdown
Collaborator

I'm not gonna ask to transpile. Here's a kind of perfect build I recommend.
https://unpkg.com/react-beautiful-dnd@11.0.4/dist/

cjs -> NODE_ENV is not replaced. Here we have react like development/production bundles. It's ok.
esm -> NODE_ENV is not replaced. It should be replaced by user bundler. No duplication for user / no shit from mixing commonjs and esm
umd -> NODE_ENV is replaced with development
min -> NODE_ENV is replaced with production and bundle minified to show kind of final size

@jaredpalmer

jaredpalmer commented Jun 12, 2019

Copy link
Copy Markdown
Owner

You cannot conditionally import ESM the way you can conditionally require in CJS like we do with the CJS entry. I confirmed this with @developit. Thus, there is no use case for a ES dev build.

@TrySound

Copy link
Copy Markdown
Collaborator

@jaredpalmer Where is the simplifity of early days rollup config? It's full of unreadable conditions now. And how you want users to build their apps with production mode messages?

@danielkcz

Copy link
Copy Markdown
Contributor Author

You cannot conditionally import ESM the way can conditionally require. I confirmed this with @developit. Thus, there is no use case for a ES dev build.

I see what you mean. That's bad... How to solve the original issue then? It breaks the development builds if a production build is used.

@TrySound

Copy link
Copy Markdown
Collaborator

Just remove replace plugin from esm pipeline and remove production/development suffix from esm bundle name.
@jaredpalmer Look, it was perfect before.
https://unpkg.com/formik@1.5.7/dist/formik.esm.js

@danielkcz

Copy link
Copy Markdown
Contributor Author

@TrySound I see what you mean. That makes sense and I assume it's the way microbundle does since it does not have such if/else logic in the output.

Oh well, should have been thinking more before diving into this PR. I suppose let's close it and do it with a clean slate.

@danielkcz danielkcz closed this Jun 12, 2019
@danielkcz

Copy link
Copy Markdown
Contributor Author

Btw, I wonder, does it actually work by default that Webpack would run replace on NODE_ENV? It's been quite a while I've configured Webpack by myself and I do remember some awkward DefinePlugin or whatnot.

@TrySound

Copy link
Copy Markdown
Collaborator

@FredyC webpack 4 has mode=production|development which uses DefinePlugin internally. It just works.

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.

Production bundle for ES format is not enough for a development

3 participants