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

Class field initialization order different between target ES2021 and ES2022 #52331

Open
DillonJettCallis opened this issue Jan 20, 2023 · 6 comments
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@DillonJettCallis
Copy link

Bug Report

🔎 Search Terms

ES2022 class field initialization order

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________

⏯ Playground Link

Playground link with relevant code

💻 Code

interface Param {
  a: number;
}

class Test {
  // constructor that sets a field
  constructor(private param: Param){
  }

  // use that field to set another field
  a = this.param.a;
}

const t = new Test({a: 10});
console.log(t.a);

🙁 Actual behavior

If the compiler is set to target "ES2021" then the output will be:

"use strict";
class Test {
    constructor(param) {
        this.param = param;
        this.a = this.param.a;
    }
}
const t = new Test({ a: 10 });
console.log(t.a);

If the target is set to "ES2022" then the output will be:

"use strict";
class Test {
    param;
    constructor(param) {
        this.param = param;
    }
    a = this.param.a;
}
const t = new Test({ a: 10 });
console.log(t.a);

These look logically the same, just utilizing the field syntax in Javascript, but in Javascript fields are initialized BEFORE the constructor is run.

So that means that ES2021 will work, setting the param field on Test and then use it to set the a field, but with ES2022 it will attempt to set a first, using the param field which has not been initialized yet, thus throwing an error at runtime.

There is no warning or error that this will happen, and I could not find any kind of compiler flag to enable such an error. Even "strictPropertyInitialization" didn't catch this like I thought it might.

🙂 Expected behavior

In order to maintain Javascript semantics, I believe that Typescript should not allow field initializers to access these automatic field setting constructor parameters. Typescript should not allow you to do something that will simply not work.

Alternatively, the compiler could be changed to insert the initializer at the end of the constructor. This would maintain existing Typescript behavior at the expense of being different from Javascript.

@whzx5byb
Copy link

Related: #50971

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experience Enhancement Noncontroversial enhancements labels Jan 24, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 24, 2023
@RyanCavanaugh RyanCavanaugh added the Help Wanted You can do this label Jan 24, 2023
@RyanCavanaugh
Copy link
Member

Alternatively, the compiler could be changed to insert the initializer at the end of the constructor

I don't think we can change the emit here, but giving a correct error under strictPropertyInitialization is definitely desirable here

@tmtron
Copy link

tmtron commented Jun 9, 2023

Alternatively, the compiler could be changed to insert the initializer at the end of the constructor

I don't think we can change the emit here, but giving a correct error under strictPropertyInitialization is definitely desirable here

So I understand that we must change our code when we want to target es2022. Unfortunately we rely on the initialization order in lots of places all over the code-base.
Do you plan to provide some sort of update-script or eslint auto-fix?

@kirbysayshi
Copy link

I'm not sure if this also falls under strictPropertyInitialization or not:

class API {
  private _state = this.initialState();

  constructor(private storage: Map<string, unknown>) {}

  initialState() {
    return this.storage.get("anything");
  }
}

This is arguably, to me at least, an example where the _state field is not being strictly initialized because it's effectively impossible under ES2022 (!). this.storage will not be set by the time this.initialState() is executed as a field. It's hard to know right now how many instances of code like this is lurking in a codebase!

Playground Link

@RyanCavanaugh
Copy link
Member

It's really unclear how you'd detect that except by checking the method bodies n times where at each n = k you pretend that only the first k class properties have been initialized. But even that doesn't really have a good way to account for what happens in base or derived methods.

@kirbysayshi
Copy link

Maybe it's better as a lint rule 🤷 ? False positives could at least be ignored then. Something about calling methods from class field properties that reference constructor shorthand-defined properties...

It's really too bad that this is how the ES2022 spec defines the execution order, as it seems to break a lot of existing TS code. For now our workaround will be "useDefineForClassFields": false, but this means that code (and our devs' mental models) will drift further and further from how ES actually executes!

huhamhire added a commit to jiandaoyun/nstarter that referenced this issue Aug 16, 2024
* ES2022 额外规定了类属性的初始化顺序行为,与 TypeScript 原有行为有差异
  - microsoft/TypeScript#52331
  - 目前通过配置 `"useDefineForClassFields": false` 保证行为的兼容性
huhamhire added a commit to jiandaoyun/nstarter that referenced this issue Aug 16, 2024
…t to feature/next

* commit '880e0864f400a56bfc571709be41f7eb6316b551':
  update: FXD-64717 解决 ES2022 属性初始化顺序问题 * ES2022 额外规定了类属性的初始化顺序行为,与 TypeScript 原有行为有差异   - microsoft/TypeScript#52331   - 目前通过配置 `"useDefineForClassFields": false` 保证行为的兼容性
  update: FXD-64717 升级 nstarter-entity -> 0.4.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants