Skip to content

Use useDefineForClassFields compiler options #186726

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

Open
jrieken opened this issue Jun 30, 2023 · 5 comments
Open

Use useDefineForClassFields compiler options #186726

jrieken opened this issue Jun 30, 2023 · 5 comments
Assignees
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.

Comments

@jrieken
Copy link
Member

jrieken commented Jun 30, 2023

Intro

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

The gist is that JavaScript now supports declaring class fields (before they were implicitly defined by assignment). In TypeScript we always declared class fields but the JS variant is slightly different. The sample below will not work anymore because TS constructor fields get now declared too.

class Foo {

    private bbb = this.aaa * 7;
                //     ^^^
                //     used BEFORE assignment 

    constructor(private aaa:number){}
}

The sample above yields a compile error because the newly emitted JS code will be this

// JS compliant code when using `useDefineForClassFields: true`

class Foo {
    aaa;
    bbb = this.aaa * 7;
      //      ^^^^^
      //      💥
    constructor(aaa) {
        this.aaa = aaa;
    }
}

The old code would have been (currently is)

class Foo {
    constructor(aaa) {
        this.aaa = aaa;
        this.bbb = this.aaa * 7;
    }
}

Why

There is some hope that explicitly declared class properties prevents shape mutations which leads to deopting

Adoption

We have ~280 compile errors when enabling useDefineForClassFields which can be fixed with ease. Move field assignment that depends on a ctor-defined-field into the ctor itself

@jrieken jrieken self-assigned this Jun 30, 2023
@jrieken jrieken added debt Code quality issues engineering VS Code - Build / issue tracking / etc. labels Jun 30, 2023
@mjbvz
Copy link
Collaborator

mjbvz commented Jul 10, 2023

Is this microsoft/TypeScript#50971? TS may not emit compile errors for all cases that useDefineForClassFields changes so you can still end up with runtime errors too

@jrieken
Copy link
Member Author

jrieken commented Jul 11, 2023

Now I am scared

@a-stewart
Copy link
Contributor

According to microsoft/TypeScript#45995 (comment), it looks like useDefineForClassFields will be deprecated in TS6 and removed in 6.5 - is this something that is on the radar?

@trevorade
Copy link
Contributor

I've started working through these errors. There are about 500 errors in this codebase when you set "useDefineForClassFields": true

I'm guessing I could resolve these in a day or two. Is this a PR you guys would be amenable to? To be clear, the PR would just be to make things compile with that setting, not actually turn it on. I expect you'd want additional testing to verify everything works as expected when emitting true JS public class fields.

jrieken added a commit that referenced this issue May 12, 2025
jrieken added a commit that referenced this issue May 12, 2025
* final init order fixes #243049

* a few more fixed when compiling with `"useDefineForClassFields": true`

#243049 and #186726

* add a dry-check that ensures we can/could compile/emit with explicitly defined class fields

#186726
jrieken added a commit that referenced this issue May 12, 2025
@jrieken
Copy link
Member Author

jrieken commented May 13, 2025

@trevorade FYI - we have finally finished #243049 and we have a "shadow compile" in CI that sets useDefineForClassFields to ensure we stay in shape.

Tho, compiling successfully and running successfully are two different pairs of shoes. I gave it a quick try yesterday and found more init-order issues that only surface during runtime. Maybe I misunderstood but are you already compiling and running with defined class fields in your fork?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.
Projects
None yet
Development

No branches or pull requests

4 participants