Skip to content

Follow ecosystem convention for __esModule on require(esm) #52134

Description

@nicolo-ribaudo

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:

// 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).

Activity

  1. GeoffreyBooth commented on Mar 18, 2024

    @GeoffreyBooth
    Member

    @nodejs/loaders

  2. nicolo-ribaudo commented on Mar 18, 2024

    @nicolo-ribaudo
    ContributorAuthor

    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.

  3. guybedford commented on Mar 18, 2024

    @guybedford
    Contributor

    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.

  4. Jarred-Sumner commented on Mar 19, 2024

    @Jarred-Sumner
    Contributor

    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)

  5. joyeecheung commented on Mar 19, 2024

    @joyeecheung
    Member

    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 import() results are not reference equal to require() results, but I guess that probably hurts nobody(?).

  6. added
    moduleIssues and PRs related to the module subsystem.
    esmIssues and PRs related to the ECMAScript Modules implementation.
    on Mar 20, 2024
  7. 2 remaining items

  8. github-actions commented on Sep 17, 2024

    @github-actions
    Contributor

    There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Issues and PRs exempt from automated stale handling. 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.

  9. added
    staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.
    on Sep 17, 2024
  10. joyeecheung commented on Sep 17, 2024

    @joyeecheung
    Member

    Fixed by #52166

  11. added a commit that references this issue on Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    esmIssues and PRs related to the ECMAScript Modules implementation.feature requestIssues requesting new Node.js features.moduleIssues and PRs related to the module subsystem.staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions