Skip to content

Dynamic imports shouldn't be transpiled in CommonJS #54111

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

Closed
Ayc0 opened this issue May 3, 2023 · 5 comments
Closed

Dynamic imports shouldn't be transpiled in CommonJS #54111

Ayc0 opened this issue May 3, 2023 · 5 comments

Comments

@Ayc0
Copy link

Ayc0 commented May 3, 2023

Bug Report

When using a dynamic import() in a TS codemod, it gets transpiled to a require() (wrapped in a Promise):

image

The issue with this is that import() are the only way for CJS files to import an ESM file. So we need to be able to keep the dynamic import in the transpiled code (because using require in those cases will crash as they aren't compatible with ESM).

Also import() is fully supported by Node, even in CJS: see https://nodejs.org/api/esm.html#import-expressions.

Dynamic import() is supported in both CommonJS and ES modules. In CommonJS modules it can be used to load ES modules.

🔎 Search Terms

esm, ecma script module, ecmascript module, common js, commonjs, cjs, dynamic import, import()

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about esm

⏯ Playground Link

Playground link with relevant code

💻 Code

import a from './a'

console.log(a)

const b = async () => {
    const a = await import('./a.mjs');
}

🙁 Actual behavior

import() is transpiled as Promise.resolve().then(() => __importStar(require('./a.mjs')))

🙂 Expected behavior

import() should transpiled as import() as dynamic imports are supported in CJS (and are the only way to import ESM from CJS)

And this, we should generate this:

"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
const a_1 = __importDefault(require("./a"));
console.log(a_1.default);
const b = async () => {
    const a = await import('./a.mjs');
};
@fatcerberus
Copy link

You need to use module: "node16" (alternatively, nodenext) to use dynamic import in CJS.

@fatcerberus
Copy link

Duplicate of #52775

@Ayc0
Copy link
Author

Ayc0 commented May 3, 2023

I don't think I can:
node16 & nodenext are using top level ESM:

  • node16:
    image
  • nodenext:
    image

What I'd expect is (roughly):

const a = require('./a');

console.log(a)

const b = async () => {
    const a = await import('./a.mjs');
}

@fatcerberus
Copy link

When using node16/next, you can name the source file .cts to get CJS output or .mts for ESM. Otherwise it’s based on your package.json IIRC

@Ayc0
Copy link
Author

Ayc0 commented May 3, 2023

Edit: in the REPL, if I switch to module: node16, and then switch back to module: commonjs, it seems to output the proper code (see in TypeScript Play)

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
const a_1 = require("./a");
console.log(a_1.default);
const b = async () => {
    const a = await import('./a.mjs');
};

This may be an issue in the REPL itself (as moduleResolution isn't exposed there)

I'll close this issue. Thanks for your reply 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants