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

Can SWC match TS emit for Parameter Properties? #9418

Closed
robpalme opened this issue Aug 11, 2024 · 6 comments · Fixed by #9421
Closed

Can SWC match TS emit for Parameter Properties? #9418

robpalme opened this issue Aug 11, 2024 · 6 comments · Fixed by #9421
Milestone

Comments

@robpalme
Copy link

Is there some config to get SWC to emit original authentic TS parameter properties?


I found that using TS parameter properties in SWC leads to JS emit that deviates from what TS produces and deviates from JS semantics. Specifically, when this TS feature is detected in a given class, SWC relocates all field initializers for that class into the constructor body.

Please note this is not about older JS versions or older TS modes. This is the behavior when not down-levelling and when ensuring useDefineForClassFields: true - which is enabled by default in TS when target: es2022 or target: esnext. So I'm asking about the most modern/minimal of output.

TypeScript Compilation

With tsc, field initializers are not relocated.

SourceTS emit
class TypeScriptParameterProperties {

    initializedField = this.pp;
    constructor(public pp: any) {

        console.log('finally');
    }
}
class TypeScriptParameterProperties {
    pp;
    initializedField = this.pp;
    constructor(pp) {
        this.pp = pp;
        console.log("finally");
    }
}

TS Playground link StackBlitz ilnk

SWC Compilation

With swc, field initializers are relocated into the constructor if (and only if) the class uses TS parameter properties.

SourceSWC emit
class TypeScriptParameterProperties {

    initializedField = this.pp;
    constructor(public pp: any) {


        console.log('finally');
    }
}
class TypeScriptParameterProperties {
    pp;
    initializedField;
    constructor(pp){
        this.pp = pp;
        this.initializedField = this.pp;
        console.log("finally");
    }
}

SWC Playground link

Conclusion

Based on history from @magic-akari it looks like this is intentional to have Swc-specific behavior. Which is Swc's choice to make - there is no spec or compliance test suite for this TS-specific feature.

For projects that want to preserve JS semantics and also want to match the official TypeScript output (e.g. in Node's experimental TS support), is there a way to do so? If not, then please consider this to be a polite feature request for a new mode, e.g. "jsc.transform.noInitializerRelocation"

@magic-akari
Copy link
Member

magic-akari commented Aug 11, 2024

I believe it's a TypeScript bug.
I meant to report it sooner but kept forgetting. Here's the issue link.


Please note this is not about older JS versions or older TS modes. This is the behavior when not down-levelling and when ensuring useDefineForClassFields: true - which is enabled by default in TS when target: es2022 or target: esnext. So I'm asking about the most modern/minimal of output.

I understand your emphasis on semantic consistency between TypeScript and JavaScript, aiming to preserve the same meaning irrespective of the presence of public keywords.
Nevertheless, we also prioritize semantic consistency across es targets. We want to ensure users don't feel confused when switching targets.

@robpalme
Copy link
Author

Thanks for the prompt and thorough explanation, @magic-akari 🙏 Raising this now with the TS team is a good way to get clarity on the stability of the transforms.

I think you describe the situation perfectly. The choice here is either consistency across ES down-level targets, or consistency between TS and JS. Today SWC favors the former and TS favors the latter.

Would you be happy accepting a PR to SWC to introduce a "jsc.transform.noInitializerRelocation" option now, or would you prefer to wait for a decision from TypeScript team on microsoft/TypeScript#59589?

@marco-ippolito
Copy link
Contributor

is there any chance we can get swc behave like TSC with an option in transformSync? When the bug is fixed by ts we flip it

@magic-akari
Copy link
Member

magic-akari commented Aug 11, 2024

I want to make one thing clear: I'm not advocating for TypeScript to alter its transform emit to align with SWC. I hope TS will resolve its own consistency problems.

Like Babel, SWC maintains consistency, but TypeScript's output is different between ES2020 and ESNEXT.

I don't believe both of these outputs are correct.

Babel has chosen an output order that more closely aligns with JavaScript semantics by omitting the public keyword, similar to TypeScript's esnext output. Conversely, SWC has selected an output style that aligns with TypeScript's ES2020 output.

Nevertheless, Babel also faces user-reported issues, as seen here: babel/babel#13779

I genuinely hope that TSC, Babel, and SWC can achieve consistent output.

Several potential solutions exist for this issue:

  1. The TypeScript team chooses a standard for output and improves consistency.
  2. The TypeScript team prioritizes stability over consistency and decides against making any changes.

If it's the first option, SWC and Babel simply need to conform to TypeScript's decision and adjust accordingly.
If it's the second, SWC will have to emulate the varying outputs of TypeScript's at different targets.

Furthermore, I believe that it is more fitting to address this issue through the TypeScript team's decision rather than adding options on our own.

@robpalme
Copy link
Author

Thanks for clarifying, @magic-akari. Apologies if I my summary was off. I updated my other comment to be more precise.

@swc-bot
Copy link
Collaborator

swc-bot commented Sep 13, 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 Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
5 participants