evanw / esbuild

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

Incorrect cyclic transpilation to cjs #1856

Open arcanis opened 2 years ago

arcanis commented 2 years ago

I spotted this issue while trying to replace babel-register w/ esbuild-register in the Yarn codebase.

x.js

import {y} from './y';

export function x() {
    y();
}

y.js

import {x} from './x';

export function y() {
    console.log(`test`);
}

x();

Running y.js after both files have been transpiled crashes with:

TypeError: (0 , import_y.y) is not a function

This is caused by __toESM, which turns the import into a new object (__create(__getProtoOf(module2))) before copying exported symbols. In the case of cyclic imports however, the exported symbols aren't ready yet when __toESM finishes executing, thus causing import_y to be an empty object:

var __toESM = (module2, isNodeMode) => {
  return __reExport(
    __markAsModule(
      __defProp(
        module2 != null
          ? __create(__getProtoOf(module2))
          : {},
        `default`,
        !isNodeMode && module2 && module2.__esModule
          ? {get: () => module2.default, enumerable: true}
          : {value: module2, enumerable: true},
      ),
    ),
    module2,
  );
};

This isn't a problem when transpiled with Babel or Webpack.

evanw commented 2 years ago

Thanks for the report. I just checked and this is not a recent regression (I suspected #1591). I can reproduce it when compiling files one-at-a-time but not when bundling, so I'm assuming you're not bundling.

This isn't a problem when transpiled with Babel or Webpack.

I'm confused. It does seem to be a problem with Babel too. Here's what I get from Babel:

And here's what I get when I run node x.js:

./x.js:11
  (0, _y.y)();
         ^

TypeError: Cannot read properties of undefined (reading 'y')
    at x (./x.js:11:10)
    at Object.<anonymous> (./y.js:14:10)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (./x.js:8:10)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)

I'm also confused about why you're mentioning Webpack. Webpack is a bundler, but this works fine with esbuild when bundling too. Are you comparing esbuild without bundling to Webpack with bundling? If Webpack works for you with bundling, then couldn't you just use esbuild with bundling instead?

arcanis commented 2 years ago

I'm also confused about why you're mentioning Webpack

Typo; I meant Typescript (via ts-node) 🙂

I'm confused. It does seem to be a problem with Babel too. Here's what I get from Babel:

We currently run Yarn in development live-from-sources using @babel/register, which transpiles files on the fly, and it's worked for a long time with the same cyclic code pattern (perhaps there are additional factors at play?).

Are you comparing esbuild without bundling to Webpack with bundling?

No, during development we don't run bundling, we just transpile the files as they get required by Node.

arcanis commented 2 years ago

Just took a quick look at your repro and there's an issue: I'm not running node x.js, I'm running node y.js. Running x.js first makes the require execute a side effect before returning, so the _y binding isn't live yet, but that's not the case when running y.js, which should work.

shishkin commented 2 years ago

I think I'm running into the same issue also when bundling. I'm troubleshooting a regression in a somewhat large TypeScript lambda function bundled and deployed via AWS CDK (that uses esbuild under the hood). I'm not sure if regression is due to updated esbuild or updated dependency that is causing the issue. However, the issue looks the same:

Esbuild generates code like this:

const func = (0, import_nth_check.default)(rule);

Which fails at runtime with:

TypeError: (0 , import_nth_check.default) is not a function
ogiekako commented 1 year ago

I hit this issue today and had to workaround it. https://crrev.com/c/4710866 This issue is hard to debug as it happens at run-time. Is any progress being made?