webpack: Broken cjs & esm interop
Bug report
What is the current behavior?
Many modules published to npm are using “auto” exports (https://rollupjs.org/guide/en#output-exports-exports, but there is also a popular babel plugin which adds this behaviour https://github.com/59naga/babel-plugin-add-module-exports#readme) which is supposed to ease interop with node (removing “pesky” .default
for CJS consumers when there is only a default export in the module).
And with that depending on a package authored solely in CJS (which still is really common) which depends on a package authored using the mentioned “auto” mode is dangerous and broken.
Why? Because webpack is using the “module” entry from package.json (thus using real default export) without checking the requester module type (which is cjs here). CJS requester did not use a .default
when requiring the package with auto mode, because from its perspective there was no such thing.
If the current behavior is a bug, please provide the steps to reproduce.
https://github.com/Andarist/webpack-module-entry-from-cjs-issue . Exported value should be "foobar42"
instead of "foo[object Module]42"
What is the expected behavior?
Webpack should deopt (ignoring .mjs & “module”) its requiring behaviour based on the requester type.
Other relevant information: webpack version: latest Node.js version: irrelevant Operating System: irrelevant Additional tools: irrelevant
Mentioning rollup team as probably its the tool outputting the most auto mode libraries ( @lukastaegert @guybedford ) and @developit (who I think might be interested in the discussion).
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 7
- Comments: 15 (8 by maintainers)
Commits related to this issue
- Use named exports for public api Default exports are always a headache for public api. They are hard to use when same module is used for both node and webpack via commonjs. We have this problem http... — committed to jonschlinkert/remarkable by TrySound 5 years ago
- Use named exports for public api (#354) Default exports are always a headache for public api. They are hard to use when same module is used for both node and webpack via commonjs. We have this prob... — committed to jonschlinkert/remarkable by TrySound 5 years ago
- Add a fallback for ESM in react-test-renderer/shallow See https://github.com/webpack/webpack/issues/7973 — committed to NMinhNguyen/react by NMinhNguyen 4 years ago
- Add a fallback for ESM in react-test-renderer/shallow See https://github.com/webpack/webpack/issues/7973 — committed to NMinhNguyen/react by NMinhNguyen 4 years ago
- build: remove babel-plugin-add-module-exports Since we have deprecated deep requires we no longer need the module.exports from the individual v*.js files. Since we now ship plain, non-babled deep re... — committed to uuidjs/uuid by ctavan 4 years ago
- build: remove babel-plugin-add-module-exports Since we have deprecated deep requires we no longer need the module.exports from the individual v*.js files. Since we now ship plain, non-babled deep re... — committed to uuidjs/uuid by ctavan 4 years ago
- build: remove babel-plugin-add-module-exports (#421) Since we have deprecated deep requires we no longer need the module.exports from the individual v*.js files. Since we now ship plain, non-babl... — committed to uuidjs/uuid by ctavan 4 years ago
- Use named exports for public api (#354) Default exports are always a headache for public api. They are hard to use when same module is used for both node and webpack via commonjs. We have this prob... — committed to bestlucky0825/remarkable by bestlucky0825 5 years ago
- Make react-icomoon work with Webpack 5 Summary: `react-icomoon` has a quirky CommonJS export with a hack meant to make it work when imported from an ES Modules context. The idea was that would set `e... — committed to CommE2E/comm by Ashoat a year ago
Having been asked to add a few cents, let me note that I have not as much experience as others in this conversation and may be overlooking things. Nevertheless I will try to shed a little more light on how rollup actually does its interop.
First let me note, which you might be aware of, that Rollup does not understand CJS internally and handles everything as ESM. If you create a CJS bundle, then Rollup’s finalizer adds some interop code which seems to be part of the discussion here.
auto mode: IMO, auto mode is pretty consistent and intuitive. It is only concerned with how the exports of a bundle, which as I stated are ESM exports internally, are mapped to
module.exports
. The logic goes like this:named
mode, otherwise we usedefault
modenamed
mode, requiring this bundle will give you an object containing the named exports, the__esModule
key, and the default export as.default
if presentdefault
mode, requiring this bundle will give you its unmodified default exportAs I said, I think it is intuitive as it enables bundles to easily export non-object results without having to worry about configuration by simply making it a default export. It is also consistent in that a bundle generated with
default
mode will look like any other npm module out there, no__esModule
flag etc. Also, there is no magic depending on exported values etc. involved as everything is handled by the static analysis.importing into CJS: Now I must admit, this is inconsistent and needs to be changed, though probably not due to the reasons listed above.
Not sure where you go the impression that rollup will behave any different for falsy default exports. I assure you it does not unless you are omitting the
default
key entirely in the exports object. Nevertheless, the interop helper is faulty, and it is in the following situation:which would be the output of
Importing this would not give you the default export but the default export key. I intend to fix this for the 1.0 by changing the interop helper to something more consistent with what Babel does, i.e. looking solely for the
__esModule
flag to detect if the default import should be taken from thedefault
key.bundled modules imported by both ESM and CJS: For rollup, this is handled solely by
rollup-plugin-commonjs
(as this is the only way to bundle CJS anyway). There is an intermediate step before bundling where the CJS modules are transformed to ESM. To distinguish between the subtleties when importing from CJS vs. importing from ESM, the imports are routed through different virtual proxy modules which all import a transpiled version of the original module. Thus, there is still only one module instance but there may be different wrapper code depending on the import type.Webpack 5 support the exports field with
require
import
andmodule
conditions, which allows every library author to choose which semantic they want to use. I recommend themodule
semantic, but one can also choose otherwise.This issue had no activity for at least three months.
It’s subject to automatic issue closing if there is no activity in the next 15 days.
There are still some packages created with babel@5 out there though, and while maybe it shouldnt be a deciding factor in this it should still be considered in making a final decision.
Sure thing, I don’t use it and I’m using
exports: 'named'
mode of rollup exclusively.Actually I would want this. User safety is IMHO far way important than some bytes added to the bundle - and I’m always trying to help libraries that I use to shave off some bytes by reviewing their build tooling and the code itself.
What I wouldn’t want is to “swallow” this silently. The best of both worlds would be IMHO to deopt behaviour for safety and at the same time print warnings about duplicated dependencies because of this, allowing people to raise issues in the repositories upstream.