evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
37.68k stars 1.11k forks source link

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

Open lmanzke opened 1 year ago

lmanzke commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 (https://github.com/evanw/esbuild/issues/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 commented 5 months ago

same issue

RobinTail commented 2 months ago

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 commented 2 months ago

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