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 fa68cbd

[a88zach]

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

[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 #5047 (comment)

[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 #5047 (comment) 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 #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 #5047 (comment), 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?

[thekip]

@dustin-rcg it's because it will transpile into

type Type<T> = T;

public myProperty: Type<TestClass>;

 ↓ ↓ ↓ ↓ ↓ ↓
__metadata("design:type", typeof Type === "undefined" ? Object : Type)

Without Type<T>

public myProperty: TestClass;

 ↓ ↓ ↓ ↓ ↓ ↓
__metadata("design:type", typeof TestClass === "undefined" ? Object : TestClass)

Note it uses Type which is not exists in runtime, normally it would use TestClass which is exists, and getter from here would be called:

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

@kdy1 could you elaborate more why this issue is considered resolved? The current implementation of SWC generates different code that TSC normally would do. And this breaks adoption of SWC in many backend projects because of this issue.

Changing the frameworks code or code in projects codebase is not a solution. swc-jest docs states that this is "drop-in replacement for ts-node" which is actually not. Just changing one to another breaks everything.

So maybe it's good idea to create an additional flag which will enable 1.2.205 transpilation behaviour?

If you need minimal repo i prepared one, but think issue is already localized and there are no need in the additional repo.

PS looking how may "Cannot access before initialization" issues appears in github this problem is worse looking IMO

[kdy1]

Your input is wrong. It's wrong because ESM is a standard

[thekip]

Well, not every framework is ready for "standard", the ecosystem right now in painful transition process and such things make it even more painful.
So having additional flag would be much appreciated. As well as documenting it somewhere. I spent more than 8 hours in total to find what was the reason it's not compiling.

[thekip]

I've created a related issue in NestJS GraphQL Repo nestjs/graphql#2568

[swc-bot]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.