Circular class imports break when upgrading 1.2.205 -> 1.2.206

[kelleyvanevert]

Describe the bug

When upgrading from 1.2.205 to 1.2.206, SWC changes how it compiles exports.

In version 1.2.205:

In version 1.2.206:

I have a bunch of model classes in my codebase, which of course circularly reference each other.

To avoid circularity problems, the ORM I use uses the "late binding" hack popular in the JavaScript community, referencing bindings with anonymous functions, e.g. @OneToMany(() => Book) instead of just @OneToMany(Book). However, this seems to no longer work after the compilation change of SWC 1.2.206.

Input code

// Person.ts
import { Book } from "./Book";

export class Person {
    books = new Collection<Book>();
}

// Book.ts
import { Person } from "./Person";

export class Book {
    author?: Person;
}

Config

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false,
      "decorators": true,
      "dynamicImport": true
    },
    "transform": {
      "legacyDecorator": true,
      "decoratorMetadata": true
    },
    "target": "es2018",
    "baseUrl": "src",
    "paths": {
      "lib/*": ["lib/*"],
      "app/*": ["app/*"],
      "fixtures/*": ["fixtures/*"],
      "test-utils/*": ["test-utils/*"]
    }
  },
  "module": {
    "type": "commonjs"
  }
}

Playground link

No response

Expected behavior

Running this code after building it, or running it with node -r @swc/register should work.

Actual behavior

Running the code after building it, or running it with node -r @swc/register, produces this error:

$ NODE_ENV=script node -r @swc/register src/fixtures/command.ts reapply
/Users/kelley/mywheels/api/src/app/core/models/Fleet.ts:6
    get: ()=>Fleet,
             ^

ReferenceError: Cannot access 'Fleet' before initialization
    at Object.get [as Fleet] (/Users/kelley/mywheels/api/src/app/core/models/Fleet.ts:16:14)
    at Object.<anonymous> (/Users/kelley/mywheels/api/src/app/core/models/FleetContractType.ts:13:16)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Module._compile (/Users/kelley/mywheels/api/node_modules/pirates/lib/index.js:99:24)
    at Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
    at Object.newLoader [as .ts] (/Users/kelley/mywheels/api/node_modules/pirates/lib/index.js:104:7)
    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)
error Command failed with exit code 1.

Version

1.2.206

Additional context

No response

[magic-akari]

The Input code you provide cannot reproduce this issue, since the Person and Book are both used as type and will be removed.

The following is equivalent esm code

// Person.js
import { Book } from "./Book";

export class Person {
    constructor(){
        this.books = new Collection();
    }
}


// Book.js
import { Person } from "./Person";

export class Book {
}

And the transformed CJS as following.

// Person.cjs
"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
Object.defineProperty(exports, "Person", {
    get: ()=>Person,
    enumerable: true
});
const _book = require("./Book");
class Person {
    constructor(){
        this.books = new Collection();
    }
}

// Book.cjs
"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
Object.defineProperty(exports, "Book", {
    get: ()=>Book,
    enumerable: true
});
const _person = require("./Person");
class Book {
}

Both work well. But you are using decorator which is more complex.

A valid minimal reproducible example is one that works fine in esm mode and crashes in the transformed cjs code.

[kdy1]

Oh sorry I missed the comment

[clhuang]

In my codebase, I've noticed that the order of the resulting imports does not seem to be the same, which can cause issues. With the following code:

export { b } from './b';
export { a } from './a';
import { q } from './a';
console.log(q);

swc 1.2.218 target ES2020 swc 1.2.218 target ES2019 swc 1.2.205 target ES2020 swc 1.2.205 target ES2019

in the first one, the resulting transpiled code imports b then a; in the latter three the resulting code imports a then b.

I've also noticed in some scenarios where a circular import would cause things to be undefined the program instead crashes, IMO this is probably preferable behavior.

[clhuang]

One example that fails under 1.2.218 but not under 1.2.205 (target ES2020) is:

// index.ts
export { default as ClassA } from './a';
export { default as ClassB } from './b';
import foo from './b';

console.log(foo);

// a.ts
import { ClassA } from '.';
export default class ClassB {
}

export const foo = 'baz';
export const bar = () => new ClassA();


// b.ts
import { ClassA } from '.';
export default class ClassB {
}

export const foo = 'baz';
export const bar = () => new ClassA();

[magic-akari]

In my codebase, I've noticed that the order of the resulting imports does not seem to be the same, which can cause issues. With the following code:

export { b } from './b';
export { a } from './a';
import { q } from './a';
console.log(q);

swc 1.2.218 target ES2020 swc 1.2.218 target ES2019 swc 1.2.205 target ES2020 swc 1.2.205 target ES2019

in the first one, the resulting transpiled code imports b then a; in the latter three the resulting code imports a then b.

I've also noticed in some scenarios where a circular import would cause things to be undefined the program instead crashes, IMO this is probably preferable behavior.

I confirmed that there is a bug in compat pass, which will be fixed soon.

One example that fails under 1.2.218 but not under 1.2.205 (target ES2020) is:

// index.ts
export { default as ClassA } from './a';
export { default as ClassB } from './b';
import foo from './b';

console.log(foo);

// a.ts
import { ClassA } from '.';
export default class ClassB {
}

export const foo = 'baz';
export const bar = () => new ClassA();


// b.ts
import { ClassA } from '.';
export default class ClassB {
}

export const foo = 'baz';
export const bar = () => new ClassA();

Tips: Rewrite import path with .js suffix and add type:module to package.json, you can run it in native node esm environment.

Transformed cjs code.

// index.js
"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
function _export(target, all) {
    for(var name in all)Object.defineProperty(target, name, {
        enumerable: true,
        get: all[name]
    });
}
_export(exports, {
    ClassA: ()=>_aJs.default,
    ClassB: ()=>_bJs.default
});
const _aJs = /*#__PURE__*/ _interopRequireDefault(require("./a.js"));
const _bJs = /*#__PURE__*/ _interopRequireDefault(require("./b.js"));
function _interopRequireDefault(obj) {
    return obj && obj.__esModule ? obj : {
        default: obj
    };
}
console.log(_bJs.default);


// a.js
"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
function _export(target, all) {
    for(var name in all)Object.defineProperty(target, name, {
        enumerable: true,
        get: all[name]
    });
}
_export(exports, {
    default: ()=>ClassB,
    foo: ()=>foo,
    bar: ()=>bar
});
const _indexJs = require("./index.js");
class ClassB {
}
const foo = 'baz';
const bar = ()=>new _indexJs.ClassA();


// b.js
"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
function _export(target, all) {
    for(var name in all)Object.defineProperty(target, name, {
        enumerable: true,
        get: all[name]
    });
}
_export(exports, {
    default: ()=>ClassB,
    foo: ()=>foo,
    bar: ()=>bar
});
const _indexJs = require("./index.js");
class ClassB {
}
const foo = 'baz';
const bar = ()=>new _indexJs.ClassA();

You can get the same result. The ClassB is printed in both case.

[stevefan1999-personal]

I have also bisected to the same version 1.2.205. After 1.2.205 this is the version that introduced this:

Object.defineProperty(exports, "<Class Name>", {
    get: ()=><Class Name>,
    enumerable: true
});

I found it because the circular imports in TypeORM tests breaks

This is probably due to https://github.com/swc-project/swc/commit/fa68cbd74ad2b36c0f1aaec563320114d5603cae

[a88zach]

For those using TypeORM, using the Relation type resolved the issue for me https://typeorm.io/#relations-in-esm-projects

[magic-akari]

It's interesting to see the linked issue. Our module transforms allow you to do circular imports, but that doesn't mean you can use before define.

This is just how import and export in JavaScript works. An import or export statement is only a declaration that the file either has dependencies or exposes live bindings to dependents. They just declare properties about the file and are not evaluated inline. At a high level, it works something like this (ignoring complexities such as top-level await):

1. Order all module files to be evaluated such that dependencies come before dependents
2. Use import and export to replace references to imports with references to the underlying exports
3. Ignore all import and export statements with paths since they have now served their purpose
4. Evaluate all code in the order in step 1

...

Originally posted by @evanw in https://github.com/evanw/esbuild/issues/1395#issuecomment-869026051

If we follow these steps to process these input (entry:index.ts)

// File one.ts
import * as entities from './'
@Entity()
export class One {
  @Column(() => entities.Two) two: entities.Two
}

// File two.ts
import * as entities from './'
@Entity()
export class Two {
  @Column(() => entities.One) one: entities.One
}

// File index.ts
import { One } from './one'
import { Two } from './two'

export default { One, Two }

We can get a single file output.

// bundle.ts
@Entity()
class One {
  @Column(() => entities.Two) two: entities.Two
}

@Entity()
class Two {
  @Column(() => entities.One) one: entities.One
}

export default { One, Two }

I am not familiar with decorators or TypeORM, but I suspect that there is also a use before define issue in this bundle.ts.

[stevefan1999-personal]

@magic-akari Actually, this is exactly what I did here

[magic-akari]

@stevefan1999-personal I may have misunderstood. The example code maybe wrong. (maybe fixed by the lazy trick) But what I'm trying to show is that the original broken code may contains use before define issue.

[stevefan1999-personal]

@stevefan1999-personal I may have misunderstood. The example code maybe wrong. (maybe fixed by the lazy trick) But what I'm trying to show is that the original broken code may contains use before define issue.

Oh sorry its me who misunderstood😂But this actually worked because the JS reference is created later

[xahhy]

Is this issue still active? we pinned the version to 1.2.205 due to this bug which we are using TypeOrm with swc

[stevefan1999-personal]

@xahhy I think this is a serious issue, as the problem stems from type system refactoring, and some fixtures aren't correctly fixed and so the right feature isn't implemented

[magic-akari]

Is this issue still active? we pinned the version to 1.2.205 due to this bug which we are using TypeOrm with swc

check https://github.com/swc-project/swc/issues/5047#issuecomment-1188874962

[powerfulyang]

谢谢大佬

Is this issue still active? we pinned the version to 1.2.205 due to this bug which we are using TypeOrm with swc

check #5047 (comment)

[Nesci28]

Same issue in a NestJS project with Circular dependancies in the DI. swc/core version to 1.2.205 does not have that bug.

Manually droping the lines:

Object.defineProperty(exports, "TestClass", {
    enumerable: true,
    get: ()=>TestClass
});

Below the line:

let TestClass = class TestClass {
...
}

Fixes the issue.

[kelleyvanevert]

Yes, @Nesci28 is correct that that defineProperty change per 1.2.206 causes the bug (whether indirectly or not).

I made a small repo with a reproduction of the bug, as it occurs in our codebase, that uses MikroORM (and reflect-metadata):

• https://github.com/MyWheels/swc_mikroorm_circular_import_bug

Indeed, I'm not sure "whose bug" this is. Whether it's swc, MikroORM, or reflect-metadata. But the combination breaks.

I'd love to resolve the situation though, because I'm currently stuck on swc 1.2.205 :)

[kdy1]

@magic-akari WIll moving export statements fix this issue? It's fine if you don't know the answer, but just to check before working on this.

[magic-akari]

No, We should not do it. We should define all the export name before any code execution.

[kdy1]

Ah, yeah I think I get the reason and you are right

[kelleyvanevert]

Before, swc would define the export name as undefined at the top of the file, and then redefine at the bottom, like so:

exports.Reservation = void 0;

...

class Reservation {
  ...
}

...

exports.Reservation = Reservation;

There must be good reasons to have changed this, but, it does seem like a compromise geared to solving exactly this situation.. ? (But I'm guessing here, i don't know :))

[magic-akari]

@kdy1 I suggest accepting https://github.com/swc-project/swc/issues/5047#issuecomment-1188874962 as the best answer. It is also accurately described in its documentation as being caused by circular references. Other decorator-based ORMs should have a similar solution.

[kdy1]

I see and ah... I think I got the exact cause. This seems like https://github.com/swc-project/swc/issues/1457

[magic-akari]

@kelleyvanevert Now swc will try to preserve ESM behavior when transform ESM to CommonJS.

You do not need ESM behavior? Try this:

exports.Reservation = class Reservation {

}

Are you using TypeScript? Try this

class Reservation {

}

export = {
    Reservation,
}

[magic-akari]

I see and ah... I think I got the exact cause. This seems like #1457

Oh, that's a terrible reason. If someone really doesn't want hoisted import, there's a TS syntax import x = require() for them.

[kelleyvanevert]

@magic-akari

It turns out that indeed, the Relation<> trick, as described in https://github.com/swc-project/swc/issues/5047#issuecomment-1188874962, indeed also works for MikroORM.

(And in fact, in most situations our codebase already uses MikroORM's IdentifiedReference<>, which effectively also solves it, I just apparently hadn't applied it everywhere yet in our codebase, and hence kept running into the bug.)

Thanks for helping!

[myknbani]

Also encountered this issue, and pinning to 1.2.205 definitely removes the Cannot access X before initialization message. Also using Nest.js. Circular deps probably the culprit.

[nerdyman]

You can also work around this by defining a type and importing it with the type keyword.

E.g.

@Entity()
export class Parent extends AggregateRoot {
  constructor() {
    super();
  }

  @PrimaryKey()
  id!: string;
}

// Declare an explicit type.
export type ParentType = Parent;

import type { ParentType } from './parent';
import { Parent } from './parent';

@Entity()
export class Child {
  @ManyToOne(() => Parent, { primary: true })
  parent!: ParentType;
}

[ticup]

Same issue here, we hard to give a reproducible example, but I'm certain it's due to circular references of classes and initialisation of those classes in the root of files.

Pinning to 1.2.205 solves it.

[dustin-rcg]

Can someone please explain why this solves the problem?

type Type<T> = T;

If all it does is indicate to swc that this is merely a type reference and not a value, then shouldn't swc be able to do this without the wrapper type? Would this be the same as what tsc analyzes regarding importsNotUsedAsValues? See also Type-Only Imports and Export.

Shouldn't swc be able to discern that these are type-only references, and do whatever magic that Type<T> does?

What makes it strange is that, in the same file, folks are using the imports as value types, but wrapped in an arrow function type => FooBar for the entity decorators. So I guess the import can't just be dropped. But then what is the magic that makes Type<T> work in this case?