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 expression not converted to require when using cjs #2651

Open
lmanzke opened this issue Nov 2, 2022 · 5 comments
Open

Dynamic import expression not converted to require when using cjs #2651

lmanzke opened this issue Nov 2, 2022 · 5 comments

Comments

@lmanzke
Copy link

lmanzke commented Nov 2, 2022

I am encountering a problem with dynamic import expressions not being rewritten.
This is a minimal reproduction example:

a.ts

import { foo } from './b';

console.log(foo);
const fun = async () => {
  const c = await import('./c');
  console.log(c);
};

fun().then(() => {
  console.log('hey');
});

b.ts

export const foo = 'hello';

c.ts

export const bar = ' world';

Running a.ts through esbuild yields the following:

esbuild --version
0.15.12
esbuild --target=node18 --format="cjs" ./a.ts

"use strict";
var __create = Object.create;
var __defProp = Object.defineProperty;
var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
var __getOwnPropNames = Object.getOwnPropertyNames;
var __getProtoOf = Object.getPrototypeOf;
var __hasOwnProp = Object.prototype.hasOwnProperty;
var __copyProps = (to, from, except, desc) => {
  if (from && typeof from === "object" || typeof from === "function") {
    for (let key of __getOwnPropNames(from))
      if (!__hasOwnProp.call(to, key) && key !== except)
        __defProp(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable });
  }
  return to;
};
var __toESM = (mod, isNodeMode, target) => (target = mod != null ? __create(__getProtoOf(mod)) : {}, __copyProps(
  isNodeMode || !mod || !mod.__esModule ? __defProp(target, "default", { value: mod, enumerable: true }) : target,
  mod
));
var import_b = require("./b");
console.log(import_b.foo);
const fun = async () => {
  const c = await import("./c");
  console.log(c);
};
fun().then(() => {
  console.log("hey");
});

Note that the import expression in the "fun" function is not transformed as opposed to the other import statement (b.ts). Either this is a bug or there is some config value that I am not aware of - however, this will basically break when it is run with node.
I would be expecting the import statement to be transformed to something like this:

  const c = await Promise.resolve().then(() => require("./c"));

(As described under https://esbuild.github.io/api/#splitting).

Note that if I run this with the --bundle flag, it yields the following output:

**esbuild --target=node18 --format=cjs --bundle ./a.ts**

"use strict";
var __defProp = Object.defineProperty;
var __getOwnPropNames = Object.getOwnPropertyNames;
var __esm = (fn, res) => function __init() {
  return fn && (res = (0, fn[__getOwnPropNames(fn)[0]])(fn = 0)), res;
};
var __export = (target, all) => {
  for (var name in all)
    __defProp(target, name, { get: all[name], enumerable: true });
};

// c.ts
var c_exports = {};
__export(c_exports, {
  bar: () => bar
});
var bar;
var init_c = __esm({
  "c.ts"() {
    "use strict";
    bar = " world";
  }
});

// b.ts
var foo = "hello";

// a.ts
console.log(foo);
var fun = async () => {
  const c = await Promise.resolve().then(() => (init_c(), c_exports));
  console.log(c);
};
fun().then(() => {
  console.log("hey");
});

Here, a transformation of the import statement takes place - however, IMHO, that should not be connected to the bundle flag.

Do I have to enable something here, pass a flag or is this something worth fixing?
Thanks in advance!

@hyrious
Copy link

hyrious commented Nov 2, 2022

First of all, unbundled mode (#708) is not supported now. Your usage is not what esbuild designed for (transform files to cjs then run them). esbuild at the first place is a bundler, which means you'd better use the build mode.

Secondly, dynamic import is supported in CJS format, here's the support table in Node.js. There's no need to lower them when it is supported. Besides, doing this transform may introduce other issues like the external file is in pure ESM which cannot be required. If you really want to lower dynamic imports in any cases, you can add --supported:dynamic-import=false.

@lmanzke
Copy link
Author

lmanzke commented Nov 2, 2022

Hey, thanks for your answer. For a bit of context, besides using esbuild indirectly by using vite, I am also trying to replace ts-jest with esbuild for faster execution speeds. I already saw an issue where people tried to do the same (#412). For my use case, it works remarkably well (vue + vite, ts, wallaby.js, esbuild and the esbuild-jest transformer). However, as already remarked in other issues, jest still has some issues with ESM, so cjs is the go-to format at the moment for running the tests.

Of course, this is not the fault of esbuild. The supported flag was indeed something I was not aware of and indeed leads to code that solves the issue I have (basically a file with a dynamic import results in a message like "You need to run with a version of node that supports ES Modules in the VM API. See https://jestjs.io/docs/ecmascript-modules").

Concerning the "unbundled mode is not supported": The jest transformer esbuild-jest seems to me like an example that relies on that being the case and I was under the impression that this is a valid use case. Are you saying that this is not how esbuild is intended to be used? The example I gave was just a stripped down variant similar to the output that is generated when running the transformer via jest. So I am not directly running this code. However, you are right that dynamic imports are indeed supported, was not aware of that! Seems that it only causes problems in conjunction with jest ...

Just a bit unfortunate that is not easy to pass flags to esbuild through the esbuild-jest transformer :/. So the supported flag cannot be communicated while transforming. I will probably have to work around that for now.

I now see the total picture clearer, thank you. I think this issue can be closed. (Not doing it for now in case somebody has something to add).

@uiolee
Copy link

uiolee commented Feb 11, 2024

same issue

RobinTail added a commit to RobinTail/express-zod-api that referenced this issue May 12, 2024
There is a Jest issue on handling dynamic imports that is really
annoying.
I used to apply an approach discussed in this issue:
evanw/esbuild#2651
But now I'm going to apply another approach from the same one, just by
disabling the dynamic imports for CJS, as a cleaner way (in my opinion).

https://esbuild.github.io/api/#supported
@RobinTail
Copy link

RobinTail commented May 12, 2024

I managed to extract an exact error message happening in Jest regarding dynamic import:

    TypeError: A dynamic import callback was invoked without --experimental-vm-modules
        at importModuleDynamicallyCallback (node:internal/modules/esm/utils:211:11)
...
    code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG'

I found a solution in disabling dynamic imports in my CJS build for compatibility with Jest.
https://esbuild.github.io/api/#supported

However, it's worth to mention that having the requested flag set Jest can process that dynamic import successfully:

yarn node --experimental-vm-modules $(yarn bin jest)

@RobinTail
Copy link

RobinTail commented May 12, 2024

So, I think it's happening because of two things:

  1. Node.js docs:

Calling import() always use the ECMAScript module loader.

Even if you're importing a CJS file that way.

  1. Jest uses a VM for running tests that does not have ECMAScript module support without the flag.

And there is a dedicated article on that:
https://nodejs.org/api/vm.html#support-of-dynamic-import-in-compilation-apis

RobinTail added a commit to RobinTail/express-zod-api that referenced this issue May 12, 2024
Still has issues with dynamic imports (disabled for CJS in #1757 )
More details on that issue I posted in
evanw/esbuild#2651 (comment)

Article on exact issue:

https://nodejs.org/api/vm.html#support-of-dynamic-import-in-compilation-apis
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