Follow ecosystem convention for `__esModule` on `require(esm)`

[nicolo-ribaudo]

What is the problem this feature will solve?

First of all, I'm incredibly excited for https://github.com/nodejs/node/pull/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:

// main.cjs
const mod = require("./dep.mjs");
console.log(Object.getOwnPropertyDescriptors(mod));

// dep.mjs
export let foo = 1;

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 whether dep.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:

import add from "add-numbers";

console.log("2 + 2 is", add(2, 2));

// add-numbers
export default (x, y) => x + y;

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)

const _addNumbers1 = require("add-numbers");
const _addNumbers = _addNumbers1.__esModule ? _addNumbers1 : { default: _addNumbers1 };

console.log("2 + 2 is", _addNumbers.default(2, 2));

// add-numbers
exports.__esModule = true;
exports.default = (x, y) => x + y;

Now, let's assume that the maintainer of add-numbers learns that Node.js now supports require(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:

import add from "add-numbers";

console.log("2 + 2 is", add.default(2, 2));

so that their existing build process transpiles it to

const _addNumbers1 = require("add-numbers");
const _addNumbers = _addNumbers1.__esModule ? _addNumbers1 : { default: _addNumbers1 };

console.log("2 + 2 is", _addNumbers.default.default(2, 2));

Their code will now work with the ESM version of add-numbers when running in Node.js, but:

• it required extra code changes
• it now won't work outside of Node.js (i.e. in bundlers), because when bundled the namespace object will be _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:

1. Cloning the namespace object: { __proto__: null, __esModule: true, ...namespace }
2. Wrapping the module in an intermediate module: export let __esModule = true; export { default } from "./that-module"; export * from "./that-module";
3. Wrapping the namespace object: Object.create(namespace, { __esModule: { value: true } })

I would personally recommend the third one, because:

• it supports live bindings (the first one does not)
• it makes __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

const _addNumbers = _addNumbers1.__esModule ? _addNumbers1 : { default: _addNumbers1 };

to

const _addNumbers = _addNumbers1.__esModule || require("util").types.isModuleNamespaceObject(_addNumbers1) ? _addNumbers1 : { default: _addNumbers1 };

However, this a few disadvantages:

• the major (more social than technical) one is that it requires every single tool out there that compiles ESM to CommonJS to be updated.
• the second (minor) one is that it makes the "was this a module?" detection CommonJS-specific. Right now tools use the same 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.
• the third one is that 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 a node:util polyfill that does not support isModuleNamespaceObject, because it's unpolyfillable).

[GeoffreyBooth]

@nodejs/loaders

• @nicolo-ribaudo How is this not a duplicate of https://github.com/nodejs/node/issues/50981? There were several concerns raised on that issue that would seem to apply here as well.

[nicolo-ribaudo]

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.

[guybedford]

An alternative isModule check that can be done here is mod[Symbol.toStringTag] === 'Module'. Adding this check alongside an __esModule check might work for both ESM externals in browsers and ESM externals in Node.js.

[Jarred-Sumner]

const _addNumbers = _addNumbers1.__esModule || require("util").types.isModuleNamespaceObject(_addNumbers1) ? _addNumbers1 : { default: _addNumbers1 };

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)

[joyeecheung]

I think when I saw https://github.com/nodejs/node/pull/51977#issuecomment-2002485317 I probably misread it as option 1 here. Option 3 definitely looks better than other alternatives. It doesn't feel as nice if import() results are not reference equal to require() results, but I guess that probably hurts nobody(?).

[github-actions[bot]]

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

[joyeecheung]

Fixed by https://github.com/nodejs/node/pull/52166