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

SWC Decorators not working for var X = @addThing class {} #8515

Closed
branks opened this issue Jan 18, 2024 · 4 comments · Fixed by #8892
Closed

SWC Decorators not working for var X = @addThing class {} #8515

branks opened this issue Jan 18, 2024 · 4 comments · Fixed by #8892
Labels
Milestone

Comments

@branks
Copy link

branks commented Jan 18, 2024

Describe the bug

We are running our code through esbuild first to generate a bundle, and then running it through swc to add the SystemJS layer on top. We have noticed that esbuild converts some classes with decorators from
@addX class C { ... } to var C = @addX class { ... } (because of evanw/esbuild#478)

This is all valid JS and works fine. However, SWC seems to fall over when then trying to handle the decorators

Input code

import { addX, addY } from 'someLib'

var C = @addX @addY class extends Component {
}

@addX @addY
class OtherClass extends Component {

}

Config

{
    "sourceMaps": true,
    "inlineSourcesContent": true,
    "jsc": {
      "target": "es2016",
      "loose": false,
      "externalHelpers": false,
      "keepClassNames": true,
      "parser": {
        "syntax": "ecmascript",
        "jsx": true,
        "decorators": true,
        "dynamicImport": true
      },
      "transform": {
        "legacyDecorator": true,
        "useDefineForClassFields": true,
        "react": {
          "refresh": false,
          "runtime": "automatic"
        }
      }
    },
    "module": {
       "type": "systemjs"
    }
  }

Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.3.104&code=H4sIAAAAAAAAA32MsQqAIBgG9%2F8pvs2lVwgC16A1R8s%2FElJDJYLw3SvbW266O%2Bv2EDMuaGPG5qVCwRKDg0jBcW8nQUSHjpBo0b1apcK86ZTAZ2ZvEmR4Tp798yIqRNWsVPSZQ145yp%2FoBoq%2FzceMAAAA&config=H4sIAAAAAAAAA22QvW7DMAyE9zxFoDlD26FD1wRBO7RLn4CQz6lS%2FYGkARuB372ykhhxUi2C7qOOR55W63KMpI4tPimLeVsrd9icdRe9i%2FiuVLYpKqIuK45ii3Cqj%2FJU4gOmEgN5eXp%2BNZsr8SkJCmjJC2YVvYIj%2BXf4DJYH%2FgvkrSeRLwpYhis0Ewv4pv80yhCV%2BprABhLLLuucogbu72yK2MAmJk1832JiQ6Tg7EfIia%2FDX%2Bg4J1GmKG3isAzjcSA77K7uj%2BadYIe27HifuM65d%2FDNPykYZHVhXsWWIT%2F3WzuzLqoL08YNdZoCqbNmLhhXt%2FdlDhNS03ncdjE65OohgyjCUc4W07fxD7fu8VM8AgAA

SWC Info output

No response

Expected behavior

I expect the output to contain the following code

 execute: function() {
            C = _class = class _class extends Component {
            };
            C = _ts_decorate([
                addX,
                addY
            ], C);
            OtherClass = class OtherClass extends Component {
            };
            OtherClass = _ts_decorate([
                addX,
                addY
            ], OtherClass);
        }

Actual behavior

Instead it seems to contain

  execute: function() {
            C = _class = class _class extends Component {
            };
            _class = _ts_decorate([
                addX,
                addY
            ], _class);
            OtherClass = class OtherClass extends Component {
            };
            OtherClass = _ts_decorate([
                addX,
                addY
            ], OtherClass);
        }

specifically

_class = _ts_decorate([
                addX,
                addY
            ], _class);

as it is applying the decorator to _class and not to C - so the class is exported and usable as C but it doesnt have the decorators applied to it

Version

1.3.104

Additional context

No response

@magic-akari
Copy link
Member

magic-akari commented Mar 4, 2024

Investigation:
The implementation of legacy Decorators in SWC follows experimentalDecorators in TypeScript.

In TypeScript, decorators can only be applied to class declarations and not to class expressions.
I haven't found relevant documentation in TypeScript yet.
My speculation is that it might be detailed in some of the TC39 specification documents.

Here is a TypeScript playground where this issue can be verified.

https://www.typescriptlang.org/play?experimentalDecorators=true#code/PTAEAEFMA8AdIE4EsC2kB2AXAhgGwCKQDGA9gtpmQM4BcomCArpAFCqxmagDeo2AJvwAaAGj6CAmqAC+oAGYISKUAHIqSyABkkAIxUsWAN2wJQAYVABeCAOE3JoIrmxUqoGJgz83ZpR3QYXNws0gbgtkL2-BIsTi5uAPKYABaIZs6u7tCe6N7mfiQBWDwG0kA

You may notice an error in the fourth line stating Decorators are not valid here.

@magic-akari
Copy link
Member

However, in Babel, it works.
I have a straightforward way to make it “just work” in SWC.

I will give it a try.

@branks
Copy link
Author

branks commented Apr 25, 2024

@magic-akari did your test work?

kdy1 pushed a commit that referenced this issue Apr 27, 2024
@kdy1 kdy1 modified the milestones: Planned, v1.5.1 Apr 27, 2024
kdy1 pushed a commit that referenced this issue May 1, 2024
**Related issue:**

Follow up #8515 

 - Ts Decorator does not always return a class; we need to give it the correct reference.
@swc-bot
Copy link
Collaborator

swc-bot commented May 27, 2024

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.

@swc-project swc-project locked as resolved and limited conversation to collaborators May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

4 participants