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

Clarify class property initializer behavior #37640

Open
pandamicro opened this issue Mar 27, 2020 · 3 comments
Open

Clarify class property initializer behavior #37640

pandamicro opened this issue Mar 27, 2020 · 3 comments
Labels
Docs The issue relates to how you learn TypeScript

Comments

@pandamicro
Copy link

pandamicro commented Mar 27, 2020

The problem

Briefly, if a class property isn't initialized with the declaration, it will be initialized as undefined, no matter how you initialize it in constructor. This will lead to v8 performance issue because of property type transition in many cases.

So I think it's very important to mention the behavior difference in documentation.

Documentation difference

In Class field declarations proposal from TC39, it's very clearly mentioned that "Fields without initializers are set to undefined."

Thus in TS handbook, there is nowhere mentioned how the field declaration without initializer will be handled. Actually the documentation lead us to believe it's the same result to initialize a property in declaration or in constructor. E.G., in the Classes default example, it says:

In the last line we construct an instance of the Greeter class using new. This calls into the constructor we defined earlier, creating a new object with the Greeter shape, and running the constructor to initialize it.

But actually it's like the TC39 proposal, if you don't assign a default value in declaration, the property will be initialized as undefined in constructor (before your actual initialize code).

Take a look at a simple example

class Test {
    x: number;
    constructor(x: number) {
        this.x = x;
    }
}

With tsc, this will be generated as the following js

"use strict";
var Test = /** @class */ (function () {
    function Test(x) {
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        this.x = x;
    }
    return Test;
}());

As you can see, property this.x is firstly defined with void 0 (undefined) and then assigned with x parameter.

Why should we care

In the previous example, there is a type transition for the property x which is sadly not a good idea in v8. I assume it cause the Test instances to be polymorphic or it cause the Test instances generate a new hidden class. Anyway, the property access performance drops significantly after the construction.

Test case:
index.js.zip

From the simple test case with a loop of Vec3 calculations, we can see the performance difference

  1. Initialize in declaration(Vec3_104): 4-5ms
  2. Initialize in constructor(Vec3_110): 28-32ms

From more complexe use case in our engine's particle system simulation, the fps drops from 60fps to 30fps.

Conclusion

I'd like to suggest a very clear description of different initializer behaviors in TypeScript documentation, so that developers know that they really need to initialize in declaration.

If possible, we'd like TSC to improve the logic:

If developers are using constructor to initialize properties, then use that value for defineProperty directly, or use a type compatible value for defineProperty.

We have also submit an issue to babel which have similar behavior.

@IllusionMH
Copy link
Contributor

IllusionMH commented Mar 27, 2020

If possible, we'd like TSC to improve the logic:

Just in case - you can avoid creation of undefined initializer (when useDefineForClassFields is set to true) by simply adding declare before class property.

class Test {
    declare x: number;
    constructor(x: number) {
        this.x = x;
    }
}

output

class TestDeclare {
    constructor(x) {
        this.x = x;
    }
}

Example

UPD. Babel also should support declare if allowDeclareFields enabled.

@RyanCavanaugh RyanCavanaugh added the Docs The issue relates to how you learn TypeScript label Mar 27, 2020
@pandamicro
Copy link
Author

pandamicro commented Mar 27, 2020

Thanks for the reply, indeed, we are completing our coding style guide by suggesting either assign a default value or use declare. Though I do think TypeScript doc should mention the exact behavior of different usages.

And I think it's better to solve such problems during tsc compile process instead of handling it with JIT at runtime

@falsandtru
Copy link
Contributor

I want TypeScript to add the flag to obey the spec of ES class fields rather than to document the difference. Can anyone create the feature request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs The issue relates to how you learn TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants