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

Incorrect cyclic transpilation to cjs #1856

Open
arcanis opened this issue Dec 15, 2021 · 5 comments
Open

Incorrect cyclic transpilation to cjs #1856

arcanis opened this issue Dec 15, 2021 · 5 comments

Comments

@arcanis
Copy link

arcanis commented Dec 15, 2021

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
Copy link
Owner

evanw commented Dec 20, 2021

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:

  • x.js

    "use strict";
    
    Object.defineProperty(exports, "__esModule", {
      value: true
    });
    exports.x = x;
    
    var _y = require("./y");
    
    function x() {
      (0, _y.y)();
    }
  • x.js

    "use strict";
    
    Object.defineProperty(exports, "__esModule", {
      value: true
    });
    exports.y = y;
    
    var _x = require("./x");
    
    function y() {
      console.log(`test`);
    }
    
    (0, _x.x)();

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
Copy link
Author

arcanis commented Dec 20, 2021

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
Copy link
Author

arcanis commented Dec 20, 2021

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
Copy link

shishkin commented Jun 2, 2022

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
Copy link

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?

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