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

keepClassNames break class decorators #6515

Open
falkenhawk opened this issue Nov 26, 2022 · 0 comments
Open

keepClassNames break class decorators #6515

falkenhawk opened this issue Nov 26, 2022 · 0 comments
Labels
Milestone

Comments

@falkenhawk
Copy link

Describe the bug

Hi, thanks for this great library and providing continuous support for it.
I have experienced an issue which I have been debugging for days.

jsc.keepClassNames breaks references, when class is decorated.

For example, creating new instance of a class, calling static method, uses the non-decorated class instead of the decorated one.

Here I try to provide the most concise and minimal reproduction I was able to come up with.

Input code

function CommonProp() {
  return <T extends { new (...args: any[]): {}; }>(Base: T) => {
    return class extends Base {
      public prop = 100;
    };
  };
}

@CommonProp()
class ClassX {
  public static checkX() {
    console.log(String(ClassX));
    console.log(new ClassX());
    // @ts-ignore
    console.log('class prop:', new ClassX().prop);
  }
}

ClassX.checkX();

Config

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "decorators": true
    },
    "target": "es2020",
    "keepClassNames": true
  }
}

Playground link

https://play.swc.rs/?version=1.3.19&code=H4sIAAAAAAAAA2WPwWrDMBBE7wb%2Fw9wiQaukV7sJofmBQnMIlB5UVXFNnZWRZNpi9O%2BNurEJ5LILmtmnmeNAJraOsHOnk6Nn73ohMZYF4G0cPOFxD%2FsTLX0EjCD7DaGU0r4JFTT9vr7JCmOqkTbiSQdbYS%2Bx3jBhZphOhzBjsm8yAP3w3rUG%2FflnrPGwWtWspP%2BdZyqLstheBywLJu7yPDDrwglRx%2FMyn9Z8HaYqgHEUXGdV5xrxEn1LjeBjKetbR67Jspj15RLbGO7bhpy3tycLTpRrVIs7XBNUfmRMurRhSU0p6z%2Fl5TWZhwEAAA%3D%3D&config=H4sIAAAAAAAAAzXKOwqAMBAFwLu82kIs09p7hyU%2BxG%2FC7gqG4N3Vwm6KqVgsIlRkUaN%2BsnK4XAjwkmlR5%2BxoMDImFU9qCK4n7wYuOtHfSOvarn3TSuZ%2BE7NBdv7xfgBpgK6LZAAAAA%3D%3D

Expected behavior

Class decorators + keepClassNames should work properly in compiled code.

Works in tsc:

❯ tsc
❯ node index.js
class extends Base {
            constructor() {
                super(...arguments);
                this.prop = 100;
            }
        }
ClassX { prop: 100 }
class prop: 100

Actual behavior

❯ npx swc index.ts -o index.js
Successfully compiled 1 file with swc.
❯ node index.js
class ClassX {
    static checkX() {
        console.log(String(ClassX));
        console.log(new ClassX());
        // @ts-ignore
        console.log('class prop:', new ClassX().prop);
    }
}
ClassX {}
class prop: undefined

Version

1.3.19

Additional context

This issue affects all devs using ts-node + swc, with target >=es2015, and also @swc-node/register, as they do not provide a way to disable keepClassNames (they ignore .swcrc file and instead transform tsconfig.json to a set of options passed directly to swc, together with swcrc: false) But still, if there was an option to disable keepClassNames, it could then break i.a. TypeORM which requires it to be enabled. (related: TypeStrong/ts-node#1343)

More tests:

Works when keepClassNames: false but of course original class name is then altered.

❯ npx swc index.ts -o index2.js
Successfully compiled 1 file with swc.
❯ node index2.js
class extends Base {
            constructor(...args){
                super(...args);
                this.prop = 100;
            }
        }
ClassX1 { prop: 100 }
class prop: 100

difference in compiled code for swc + keepClassNames:true (non-working) vs swc + keepClassNames:false (working, but)

@@ -14,7 +14,7 @@ function CommonProp() {
         };
     };
 }
-let ClassX = class ClassX {
+let ClassX = class ClassX1 {
     static checkX() {
         console.log(String(ClassX));
         console.log(new ClassX());

difference between swc + keepClassNames:true (non-working) vs tsc (working) (ignoring formatting differences, I ran prettier on both outputs)

+'use strict';
 var __decorate =
   (this && this.__decorate) ||
   function (decorators, target, key, desc) {
@@ -17,23 +18,24 @@ var __decorate =
           r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
     return c > 3 && r && Object.defineProperty(target, key, r), r;
   };
+var ClassX_1;
 function CommonProp() {
   return (Base) => {
     return class extends Base {
-      constructor(...args) {
-        super(...args);
+      constructor() {
+        super(...arguments);
         this.prop = 100;
       }
     };
   };
 }
-let ClassX = class ClassX {
+let ClassX = (ClassX_1 = class ClassX {
   static checkX() {
-    console.log(String(ClassX));
-    console.log(new ClassX());
+    console.log(String(ClassX_1));
+    console.log(new ClassX_1());
     // @ts-ignore
-    console.log('class prop:', new ClassX().prop);
+    console.log('class prop:', new ClassX_1().prop);
   }
-};
-ClassX = __decorate([CommonProp()], ClassX);
+});
+ClassX = ClassX_1 = __decorate([CommonProp()], ClassX);
 ClassX.checkX();

Tested on node v16.4.2, win10.

Possibly also related to #5291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants