Skip to content
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

Adding an option to interpret __esModule as Babel does #40891

Closed
qnighy opened this issue Nov 20, 2021 · 2 comments
Closed

Adding an option to interpret __esModule as Babel does #40891

qnighy opened this issue Nov 20, 2021 · 2 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.

Comments

@qnighy
Copy link

qnighy commented Nov 20, 2021

Is your feature request related to a problem? Please describe.

Node.js provides interop between ES Modules and CommonJS Modules in the following ways:

  • CJS → ESM: await import("foo.mjs") works. Since CJS module body is executed synchronously, it is not possible to import an ESM module during CJS module setup.
  • ESM → CJS: an ES module can import a CJS module as usual and the CJS module is automatically wrapped in an ES module.

So basically, the migration should start from the depending side (= application code). In this strategy, each import edge in the module graph will transition in the following manner:

  • Step 1: a CJS module imports a CJS module.
  • Step 2: an ES module imports a CJS module.
  • Step 3: an ES module imports an ES module.

And it is already a widespread practice to write each package in ESM syntax and transform the modules to CJS using Babel or TypeScript (tsc). Node.js and Babel differ in how default imports are treated. If both the importing module and the imported module were in ESM syntax, the behavior will change like:

  • Step 1: a CJS module (transformed by Babel) imports a CJS module (transformed by Babel). The default import will reference the value exported by export default.
  • Step 2: an ES module imports a CJS module (transformed by Babel). The default import will reference the namespace object.
  • Step 3: an ES module imports an ES module. The default import will reference the value exported by export default.

Instead of flipping imports manually, it would be really useful if Node.js can provide a behavior compatible with Babel's behavior.

Describe the solution you'd like

To change the behavior of the CJS compatibility layer. The current behavior is as follows:

  • The value of module.exports in CJS is mapped to the default export in ESM.

I propose to change the behavior:

  • If module.exports.__esModule is truthy, the value of module.exports.default in CJS is mapped to the default export in ESM.
  • Otherwise, the value of module.exports in CJS is mapped to the default export in ESM.

In my understanding, the existing behavior was introduced in #14369. At that time, named exports were not available and the decision was reasonable because we basically want to allow users to access all functionality from CJS modules.

When #35249 was merged to allow named exports/imports, the behavior was not changed as described in the original PR: to maintain compatibility and consistency.

The major edge case this differs from current semantics with bundlers on is that the default export is retained as the full module.exports instance for backwards compat and consistency. This is important for Node.js semantic consistency I think.

I think it's more consistent to support __esModule flag as it provides an accurate embedding of ESM semantics to CJS. However backwards compat is still important; I propose to provide a command-line option to support __esModule instead.

I made a PR which contains the solution described above: #40892

Describe alternatives you've considered

  • Keep the status quo.
    • We can still configure Babel to use importInterop: "node" and fix the code appropriately. However, the user will struggle again if the dependent package migrates to full ESM.
    • TypeScript is not yet fully aware of that pattern either, as far as I understand.
    • There's also a difficulty in macro-based libraries such as styled-components/macro as they expect the specific default import pattern.
  • Change the behavior globally.
    • The change can't be landed in Node.js 16 in this approach, as it is apparently backward-incompatible.
  • Provide a finer-grained option in package.json rather than a command-line option.
    • Option 1: interpret the option in package.json for the imported module
      • It's not effective, because not all packages are frequently maintained.
    • Option 2: interpret the option in package.json for the importing module
      • It will add an extra lookup overhead for CommonJS compatibility layer.
qnighy added a commit to qnighy/node that referenced this issue Nov 20, 2021
Adds an option `--cjs-import-interop`. When enabled, a CJS module will
be translated to ESM slightly differently with respect to default
exports.

- When the module defines `module.exports.__esModule` as a truthy value,
  the value of `module.exports.default` is used as the default export.
- Otherwise, `module.exports` is used as the default export.
  (existing behavior)

It allows better interoperation between full ES modules and CJS modules
transformed from ES modules by Babel or tsc. Consider the following
example:

```javascript
// Transformed from:
// export default "Hello";
Object.defineProperty(module.exports, "__esModule", { value: true });
module.exports.default = "Hello";
```

When imported from the following module:

```javascript
import greeting from "./hello.cjs";
console.log(greeting);
```

With `--cjs-import-interop`, it will print "Hello".

Fixes: nodejs#40891
@VoltrexKeyva VoltrexKeyva added esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem. labels Nov 20, 2021
qnighy added a commit to qnighy/node that referenced this issue Nov 21, 2021
Adds an option "importInterop" to package.json. When enabled, `import`
statements in that package will behave slightly different with respect
to default exports if the imported module is CJS.

- When the imported module defines `module.exports.__esModule` as a
  truthy value, the value of `module.exports.default` is used as the
  default export.
- Otherwise, `module.exports` is used as the default export.
  (existing behavior)

It allows better interoperation between full ES modules and CJS modules
transformed from ES modules by Babel or tsc. Consider the following
example:

```javascript
// Transformed from:
// export default "Hello";
Object.defineProperty(module.exports, "__esModule", { value: true });
module.exports.default = "Hello";
```

When imported from the following module:

```javascript
import greeting from "./hello.cjs";
console.log(greeting);
```

with the following package.json:

```javascript
{
  "type": "module",
  "importInterop": true
}
```

Then it will print "Hello".

Fixes: nodejs#40891
@benjamingr
Copy link
Member

Hey, it looks like the module team consensus is to not do this so I'll go ahead and close the issue and Pr. Thanks a lot for the detailed request and implementation attempt.

If you would be interested in getting more involved there is a lot of interesting work to do in the modules space in Node. I encourage you to get involved in the discussions :)

@qnighy
Copy link
Author

qnighy commented Feb 18, 2022

Thank you for the responses too! As a (probably) final comment, let me advertise another solution for someone came across this issue: I made a Babel plugin to mitigate the problem described in this issue: https://github.com/qnighy/node-cjs-interop. Check it out if you're having the default importing issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.
Projects
None yet
3 participants