-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
Node-style default interop #10971
Comments
Just FYI - this is effectively asking for a webpack mode or flag to disable support for faux modules. faux modules is an emerging term for published modules that were transpiled to CJS + |
I won't call them such way. Flagged modules existed long before node started to think about esm support. They are a valid module format in the middle between cjs and esm. Even while there is no official spec for that, in this module format webpack has a special mode in .mjs (and type: module in future) where this flag is ignored, but I'm unhappy with the complexity and work created because node.js varys from the ecosystem standards. Same for the module field in package.json. After all this will be behind an |
This issue is specifically not about breaking any semantics for named exports of CJS in ESM. This issue is only about tracking that The This issue is not about |
Close in favor #7973 |
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 `exports.__esModule` to enable the hack, and you would export a prop named `default` that would be the "default" import for the ESM. This hack was supported in Webpack 4, but not in Webpack 5. There's a lot of [noise](webpack/webpack#10971) about [this](webpack/webpack#7973) on [GitHub](babel/babel#12363), but the Webpack team points to Node as no longer supported this hack either as justification for pulling it out. The `react-icomoon` package uses this hack. I tried upgrading it, but the latest version still had the same problem. To get around it, I just update the code to access `.default` directly. Note that this means `lib/components/SWMansionIcon.react.js` wouldn't work in a React Native context, despite `react-icomoon` claiming to support React Native. But we don't need it on React Native since we use `createIconSetFromIcoMoon` from `@expo/vector-icons` instead. Depends on D6703 Test Plan: 1. I tested dev build in `web` via `yarn dev` in `web` + `keyserver` and then tested the website 2. I tested prod build by running `yarn prod` in `web` and then tested the website by running keyserver with `yarn prod-build && yarn-prod` Reviewers: atul, tomek Reviewed By: atul Differential Revision: https://phab.comm.dev/D6704
Feature request
Currently Webpack supports some of the following interops:
where
pkg
hasexports.__esModule
andexports.default
, will import thepkg.default
property instead of returning thepkg
property directly. This is in contrast to Node.js behaviour.In addition:
should import the
pkg
property forcing users to do.default.default
in this case (and not checking__esModule
interop, as Node.js doesn't do this).Named exports are still up in the air for Node.js but would be additions to the above if this happens.
I would suggest still supporting named exports from the instance, but ensuring the
default
export interop still behaves as above.What is the expected behavior?
Correct Node.js interop semantics would be for:
to be represented by the namespace object
ModuleNamespace { default: exports }
, such that:Also when importing from one CJS module to another, the namespace default export should be used.
Node.js semantics do not govern how CJS should import ESM, so getting the full ES module namespace as the
require()
output does seem fine in this case.What is motivation or use case for adding/changing the behavior?
Ensuring smooth ecosystem behaviours between Node.js, build tools and browsers.
How should this be implemented in your opinion?
A simple flag should suffice. I do think it should be the default mode eventually.
Are you willing to work on this yourself?
If there are dev bandwidth constraints for such a feature, I can certainly see if I can find wider support.
The text was updated successfully, but these errors were encountered: