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

esm: add pjson.importInterop to support __esModule #40902

Closed
wants to merge 1 commit into from

Conversation

qnighy
Copy link

@qnighy qnighy commented Nov 21, 2021

This is another attempt at solving #40891 following the discussion starting with #40892 (comment). The previous attempt is #40892 and I made a different PR because they're different solutions and have pros and cons.

This PR 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:

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

When imported from the following module:

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

with the following package.json:

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

Then it will print "Hello".

Fixes: #40891

Unlike the previous approach, it doesn't (and doesn't intend to) change the default behavior. It still solves the problem illustrated in #40891 because only ESM → CJS boundaries are problematic and such boundaries will most likely emerge when the importing package is migrating from CJS to ESM. Even if the imported package is inactive, it is easy to configure the importing package as it is the package that is migrating to ESM.

I used getPackageScopeConfig to retrieve package.json. I think it is a reliable one because it is already used in #path/to/alias imports. I'm unsure about its performance implication, but the effect is limited to ESM → CJS edges.

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
@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Nov 21, 2021
@benjamingr
Copy link
Member

Looks like this (and the issue) have settled on a consensus not to do this so I'll go ahead and close this. If anyone feels strongly or wants to explore feel free to reopen.

Thanks for the detailed ask and good PR!

@benjamingr benjamingr closed this Feb 18, 2022
@qnighy qnighy deleted the cjs-import-interop2 branch February 18, 2022 10:00
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. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding an option to interpret __esModule as Babel does
3 participants