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

Inconsistent useDefineForClassFields behavior #6985

Closed
magic-akari opened this issue Feb 24, 2023 · 3 comments · Fixed by #7055
Closed

Inconsistent useDefineForClassFields behavior #6985

magic-akari opened this issue Feb 24, 2023 · 3 comments · Fixed by #7055
Labels
Milestone

Comments

@magic-akari
Copy link
Member

magic-akari commented Feb 24, 2023

Describe the bug

result from TypeScript:

useDefineForClassFields true false None
ES2022 Native [[Set]] Native
<= ES2021 [[Define]] [[Set]] [[Set]]

Default: true if target is ES2022 or higher, including ESNext,,false otherwise.
ref: https://www.typescriptlang.org/tsconfig#useDefineForClassFields

useDefineForClassFields: false always works in tsc, no matter what the target is.
However, if this option is missing, the tsc result will be different when the target changes.
The behavior of the Native class field is [[Define]].

The behavior of swc and tsc is not identical, and the JS and TS outputs of swc are not identical.

Ref: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#the-usedefineforclassfields-flag-and-the-declare-property-modifier

Input code

class Foo {
  x = 1;
}

Config

No response

Playground link

No response

Expected behavior

useDefineForClassFields true false None
ES2022 Native [[Set]] Native
<= ES2021(TS) [[Define]] [[Set]] x
<= ES2021(JS) [[Define]] [[Set]] x
  1. We expect useDefineForClassFields: false to always work.
  2. We expect the same results from JS and TS.

x can be either [[Define]] or [[Set]].
If we choose [[Set]], then we keep the same behavior as TypeScript.
If we choose [[Define]], then we get consistency with our None option, since Native means [[Define]].

Actual behavior

useDefineForClassFields true false None
ES2022 Native Native Native
<= ES2021(TS) [[Define]] [[Set]] [[Set]]
<= ES2021(JS) [[Define]] [[Define]] [[Define]]
  1. useDefineForClassFields: false does not work for ES2022.
  2. The results for useDefineForClassFields: false are different for TS and JS.
  3. The results for useDefineForClassFields: None are different for TS and JS.

Version

1.3.36

Additional context

Output 1: Native class field

class Foo {
    x = 1;
}

Output 2: [[Set]]

class Foo {
    constructor() {
        this.x = 1;
    }
}

Output 3: [[Define]] from TypeScript

class Foo {
    constructor() {
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: 1
        });
    }
}

Output 4: [[Define]] from swc

function _defineProperty(obj, key, value) {
    if (key in obj) {
        Object.defineProperty(obj, key, {
            value: value,
            enumerable: true,
            configurable: true,
            writable: true
        });
    } else {
        obj[key] = value;
    }
    return obj;
}
class Foo {
    constructor(){
        _defineProperty(this, "x", 1);
    }
}
@kdy1 kdy1 added this to the Planned milestone Feb 24, 2023
@magic-akari
Copy link
Member Author

What do you think should be done to fix this issue? @kdy1

@kdy1
Copy link
Member

kdy1 commented Feb 25, 2023

I think we can change the default value based on the target, but for first I think we should make behavior of js and ts identical

kdy1 pushed a commit that referenced this issue Mar 12, 2023
**BREAKING CHANGE:**

IMPORTANT NOTE: Users of decorators are recommended to configure `"useDefineForClassFields": false` to ensure that your code is properly transpiled.


**Related issue:**
 - Closes #6985.
@kdy1 kdy1 modified the milestones: Planned, v1.3.40 Mar 13, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented Apr 12, 2023

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 Apr 12, 2023
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.

3 participants