-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Follow ecosystem convention for __esModule
on require(esm)
#52134
Comments
@nodejs/loaders
|
They are for the two opposite directions: that one is for importing CJS, this one is for requiring ESM. None of the concerns listed there apply here, because the concerns there are about compatibility with older Node.js versions. |
An alternative |
If I'm understanding correctly, this approach adds extra work at each re-export site. IMO, it's really important to not make apps take longer to start. For apps that have lots of exports, this will make apps start slower, especially in files with a large number of exports (like barrel files / icons) |
I think when I saw #51977 (comment) I probably misread it as option 1 here. Option 3 definitely looks better than other alternatives. It doesn't feel as nice if |
PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Tooling in the ecosystem have been using the __esModule property to recognize transpiled ESM in consuming code. For example, a 'log' package written in ESM: export function log(val) { console.log(val); } Can be transpiled as: exports.__esModule = true; exports.default = function log(val) { console.log(val); } The consuming code may be written like this in ESM: import log from 'log' Which gets transpiled to: const _mod = require('log'); const log = _mod.__esModule ? _mod.default : _mod; So to allow transpiled consuming code to recognize require()'d real ESM as ESM and pick up the default exports, we add a __esModule property by building a source text module facade for any module that has a default export and add .__esModule = true to the exports. We don't do this to modules that don't have default exports to avoid the unnecessary overhead. This maintains the enumerability of the re-exported names and the live binding of the exports. The source of the facade is defined as a constant per-isolate property required_module_facade_source_string, which looks like this export * from 'original'; export { default } from 'original'; export const __esModule = true; And the 'original' module request is always resolved by createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping over the original module. PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Tooling in the ecosystem have been using the __esModule property to recognize transpiled ESM in consuming code. For example, a 'log' package written in ESM: export function log(val) { console.log(val); } Can be transpiled as: exports.__esModule = true; exports.default = function log(val) { console.log(val); } The consuming code may be written like this in ESM: import log from 'log' Which gets transpiled to: const _mod = require('log'); const log = _mod.__esModule ? _mod.default : _mod; So to allow transpiled consuming code to recognize require()'d real ESM as ESM and pick up the default exports, we add a __esModule property by building a source text module facade for any module that has a default export and add .__esModule = true to the exports. We don't do this to modules that don't have default exports to avoid the unnecessary overhead. This maintains the enumerability of the re-exported names and the live binding of the exports. The source of the facade is defined as a constant per-isolate property required_module_facade_source_string, which looks like this export * from 'original'; export { default } from 'original'; export const __esModule = true; And the 'original' module request is always resolved by createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping over the original module. PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Tooling in the ecosystem have been using the __esModule property to recognize transpiled ESM in consuming code. For example, a 'log' package written in ESM: export function log(val) { console.log(val); } Can be transpiled as: exports.__esModule = true; exports.default = function log(val) { console.log(val); } The consuming code may be written like this in ESM: import log from 'log' Which gets transpiled to: const _mod = require('log'); const log = _mod.__esModule ? _mod.default : _mod; So to allow transpiled consuming code to recognize require()'d real ESM as ESM and pick up the default exports, we add a __esModule property by building a source text module facade for any module that has a default export and add .__esModule = true to the exports. We don't do this to modules that don't have default exports to avoid the unnecessary overhead. This maintains the enumerability of the re-exported names and the live binding of the exports. The source of the facade is defined as a constant per-isolate property required_module_facade_source_string, which looks like this export * from 'original'; export { default } from 'original'; export const __esModule = true; And the 'original' module request is always resolved by createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping over the original module. PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
PR-URL: nodejs#52166 Refs: nodejs#52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Tooling in the ecosystem have been using the __esModule property to recognize transpiled ESM in consuming code. For example, a 'log' package written in ESM: export function log(val) { console.log(val); } Can be transpiled as: exports.__esModule = true; exports.default = function log(val) { console.log(val); } The consuming code may be written like this in ESM: import log from 'log' Which gets transpiled to: const _mod = require('log'); const log = _mod.__esModule ? _mod.default : _mod; So to allow transpiled consuming code to recognize require()'d real ESM as ESM and pick up the default exports, we add a __esModule property by building a source text module facade for any module that has a default export and add .__esModule = true to the exports. We don't do this to modules that don't have default exports to avoid the unnecessary overhead. This maintains the enumerability of the re-exported names and the live binding of the exports. The source of the facade is defined as a constant per-isolate property required_module_facade_source_string, which looks like this export * from 'original'; export { default } from 'original'; export const __esModule = true; And the 'original' module request is always resolved by createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping over the original module. PR-URL: nodejs#52166 Refs: nodejs#52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the
never-stale
|
Fixed by #52166 |
PR-URL: #52166 Refs: #52134 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
What is the problem this feature will solve?
First of all, I'm incredibly excited for #51977 :) It's great to finally see the biggest pain point when using Node.js being addressed.
However, Node.js is not the first tool to implement
require(esm)
and it would be great if it could follow the existing ecosystem convention. Given this code:Webpack, Rollup, Parcel, esbuild and Bun will all log an object with an
__esModule: true
property. This convention makes sure that the code will behave the same whetherdep.mjs
runs as native ESM or as ESM-compiled-to-CJS.While this doesn't really matter when the CJS and ESM files are in the same project, it's very helpful at the boundary between one library and another. Consider this case:
When running as ESM, this obviously logs
2 + 2 is 4
. When running as ESM-compiled-to-CJS, this also logs the same result because the two files are compiled to (roughly)Now, let's assume that the maintainer of
add-numbers
learns that Node.js now supportsrequire(esm)
, and decides to release the new version of their library as ESM since this won't have anymore the annoying effect of forcing its consumers to migrate to ESM.If their user update to the new version, when running in Node.js their code will now stop working, throwing something like
_addNumbers.default is not a function
(because_addNumbers1
is not the module namespace object). Instead, they have to change their code to this:so that their existing build process transpiles it to
Their code will now work with the ESM version of
add-numbers
when running in Node.js, but:_addNumbers
and not_addNumbers.default
Note that Node.js already has this problem when migrating to ESM the other way around (i.e. when migrating the consumer before the library), but it is for reasonable historic reasons (that is, the original Node.js CJS-ESM integration was just "assign
module.exports
to the default export" without trying to make CJS being importable as if it was an ESM file with various exports), and it looks like it would be unfixable even through some opt-in mechanism.What is the feature you are proposing to solve the problem?
Instead of returning the module namespace as-is, Node.js should wrap it in something that adds an
__esModule: true
property to it. There are three ways of doing it:{ __proto__: null, __esModule: true, ...namespace }
export let __esModule = true; export { default } from "./that-module"; export * from "./that-module";
Object.create(namespace, { __esModule: { value: true } })
I would personally recommend the third one, because:
__esModule
non-enumerable (the second one does not)This wrapping could either be done unconditionally, or only if the module doesn't already have an
__esModule
export (which is very rare, as it's incompatible with all tools that compile ESM to CJS).What alternatives have you considered?
Joyee suggested to instead update the detection in tools from
to
However, this a few disadvantages:
require()
wrapper when compiling to CommonJS, AMD or UMD -- CommonJS and AMD/UMD would now need different detections to accomodate the difference of one of the platforms with ESM-CJS interop.require("util").types.isModuleNamespaceObject
is Node.js-specific. That check will not work when bundled and sent to the browser, unless users figure out how to polyfill Node.js builtin modules in their tool (and still, googling for "webpack require util not found" leads to a StackOverflow answer that suggests anode:util
polyfill that does not support isModuleNamespaceObject, because it's unpolyfillable).The text was updated successfully, but these errors were encountered: