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

Do not differ between CommonJS and ESM builds #2571

Closed
101arrowz opened this issue Nov 8, 2020 · 8 comments
Closed

Do not differ between CommonJS and ESM builds #2571

101arrowz opened this issue Nov 8, 2020 · 8 comments

Comments

@101arrowz
Copy link

What problem does this feature solve?

Parcel 2 breaks on Vue with HMR because it expects __VUE_HMR_RUNTIME__ to be defined in the global scope. This is only defined in the ESM build because Vue assumes all bundlers will use ESM. However, to save Babel compilation time in development, Parcel 2 uses the CommonJS version, defined by "main" in package.json, meaning the HMR runtime added by the Vue transformer breaks. This issue was found in parcel-bundler/parcel#5325.

What does the proposed API look like?

Code that functions differently with CommonJS and ESM is unusual and is arguably an antipattern. Creating a CJS version for bundlers and adding that to "main" or adding the HMR runtime to the existing CJS build would fix the issue.

@posva
Copy link
Member

posva commented Nov 9, 2020

This sounds more like a bug in Parcel: they should follow the same convention followed by webpack and include the module instead of main one when doing bundling. That way they can benefit from tree shaking and any other bundler-only feature. The CJS version is meant to be used on Node

Code that functions differently with CommonJS and ESM is unusual and is arguably an antipattern.

No, because both target different environments (Node vs Browser) so it makes sense to optimize them differently

@101arrowz
Copy link
Author

Tree shaking doesn't work with HMR, hence ESM is useless when developing with HMR and transpiling ESM to CommonJS, which is ultimately used in the final build, only slows the build down.

You can't assume ESM is bundler and CJS is Node anymore; Node supports ESM loading. Instead, the "browser" field of package.json should being used to do that.

If you really want to force Parcel to unnecessarily transpile ESM in development, setting the "browser" field to the ESM version will fix the issue.

@mischnic
Copy link

mischnic commented Nov 9, 2020

If you really want to force Parcel to unnecessarily transpile ESM in development, setting the "browser" field to the ESM version will fix the issue.

👍


(The modern way for different builds for esm/cjs/node is the exports field: https://webpack.js.org/guides/package-exports/#target-environment.)

@101arrowz
Copy link
Author

@posva Would adding the "browser" field to package.json be possible? It just needs to refer to the same thing as "module" and would fix Parcel 2 support.

@posva
Copy link
Member

posva commented Nov 12, 2020

If I recall correctly, using "browser" for the ESM breaks other things because it's not a browser module, it contains environment variables

@101arrowz
Copy link
Author

As far as I know, only bundlers support the "browser" key, and since bundlers can replace the environment variables, it should work...

@posva
Copy link
Member

posva commented Nov 12, 2020

No, bundlers are not the only ones, some cdns use it too

@mischnic
Copy link

One solution would be adding typeof process !== "undefined" when the env variables are checked.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants