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

Dynamic import to CommonJS does not compile to require #1540

Closed
lxsmnsyc opened this issue Aug 22, 2021 · 6 comments
Closed

Dynamic import to CommonJS does not compile to require #1540

lxsmnsyc opened this issue Aug 22, 2021 · 6 comments

Comments

@lxsmnsyc
Copy link

lxsmnsyc commented Aug 22, 2021

There seems to be a regression that happened between 0.9.5 and 0.12.22.

When bundling ESM code to Node(platform: "node" and format: "cjs"), the output code seems to be unchanged.

Output:

async function lazyCode() {
  const mod = await import('path');
}

Expected:

async function lazyCode() {
  const mod = await Promise.resolve().then(() => toModule(require('path')));
}

edit: looks like this happened on 0.10.0 when import and require behavior has been changed.

@hyrious
Copy link

hyrious commented Aug 23, 2021

Dynamic import() is available since node13.2. So if you need it to be transformed to require(), use --target=node12.

@lxsmnsyc
Copy link
Author

Dynamic import() is available since node13.2. So if you need it to be transformed to require(), use --target=node12.

Even though it is supported in Node 13.2 and above, the code only runs for packages with "type": "module". The intended behavior when format: "cjs" is to always convert import statements to require, since CommonJS, regardless of the Node version, do not support import statements. Currently, this conversion only works for static imports and it used to be the case for the dynamic import expressions.

@clydin
Copy link

clydin commented Aug 23, 2021

Dynamic import expressions are supported in both CJS and ESM (https://nodejs.org/docs/latest-v14.x/api/esm.html#esm_import_expressions).
This is particularly useful when ESM code needs to be used from within CJS code.

@lxsmnsyc
Copy link
Author

In CommonJS modules, it can be used to load ES modules

Now that's the behavior I never wanted. I'll try readjusting the node target then

@evanw
Copy link
Owner

evanw commented Nov 21, 2021

I'm going to close this because this is by design. Since import() and require() behave differently, it would be wrong to convert one into the other (unless of course you have to because it's unsupported and there's no other option). These are the differences I'm aware of:

  • Doing import() on an ES module works but doing require() on an ES module doesn't work (described above).

  • When resolving exports in package.json, import() uses the import condition while require() uses the require condition so using these two expressions with the same path can easily end up loading completely different files. You can learn more about this here: https://nodejs.org/api/packages.html#conditional-exports.

@evanw evanw closed this as completed Nov 21, 2021
@lxsmnsyc
Copy link
Author

Yes thank you @evanw, I went with node12 target so that it won't preserve the dynamic import.

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

4 participants