Skip to content

Strict property initialization (arguably) confused by redefinition of property in subclass #21775

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

Closed
ethanresnick opened this issue Feb 8, 2018 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ethanresnick
Copy link
Contributor

ethanresnick commented Feb 8, 2018

TypeScript Version: 2.7.1

Search Terms:
property initialized used sub class

Code

class Super {
    protected prop: { a: number; };
    constructor() {
        this.prop = { a: 1 };
    }
}

class Sub extends Super {
    protected prop: { a: number, b: number };

    constructor() {
        super();
        this.prop = {
            ...this.prop,
            b: 2
        };
    }
}

Expected behavior:
The assignment to this.prop in the subclasss' constructor should succeed, with the compiler understanding that this.prop, at that stage, has the value + type given to it by the superclass.

Actual behavior:
Error: Property 'prop' is used before being assigned.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 8, 2018

//related #20911

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Feb 8, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Feb 8, 2018

I am not sure there is anything we can do here.. the compiler sees a reference to this.props, but at that point it does not know what it will be used for..

The workaround is to add the definite assignment operator on the declaration, e.g. protected prop!: { a: number, b: number };

@ethanresnick
Copy link
Contributor Author

ethanresnick commented Feb 8, 2018

Yeah, this is very similar to #20911, with the difference that we actually are assigning to the property in the subclass constructor, so there's not that complication around whether just defining the field in the subclass would create an undefined own prop.

the compiler sees a reference to this.props, but at that point it does not know what it will be used for..

What do you mean by "what it will be used for"/what's the relevance of that? It seems the compiler should know the type this.props on the right-hand side of the initial assignment, and that seems like enough.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 9, 2018

What do you mean by "what it will be used for"/what's the relevance of that?

Any use of this.props outside the initialization of itself is invalid, since this.prop.b will be undefined. the compiler does not know when it is looking at ...this.prop that it is meant to be spread in this.prop.

@ethanresnick
Copy link
Contributor Author

ethanresnick commented Feb 9, 2018

Any use of this.props outside the initialization of itself is invalid, since this.prop.b will be undefined. the compiler does not know when it is looking at ...this.prop that it is meant to be spread in this.prop.

Right, I guess I'm saying rule should be that:

  1. this.prop must still be initialized in the subclass' constructor before that constructor finishes, using the type the subclass defines for this.prop. (If this.prop.b were optional, then we'd be in the strictPropertyInitialization with subclasses that refine the types of properties #20911 case, but let's ignore that for now.)

  2. Because the compiler is tracking when this.prop is first assigned to in the subclass' constructor, it should be able to tell which reads of this.prop are before that first assignment. For all such reads, it would use the parent class' type for this.prop. It doesn't have to know that the read is being used to set the child class's this.prop. (If the parent class doesn't define the prop, then the read would be invalid, as now.)

So the following would be valid too under my proposal:

class Super { /* Same as before */ }

class Sub extends Super {
    protected prop: { a: number, b: number };
    protected derivedProp: number;

    constructor() {
        super();
        // below would be a totally legal read of this.prop, even though it's not 
        // used to set the child's this.prop. Uses the parent class's type 
        // (i.e., so a read of this.prop.b here would be invalid).
        this.derivedProp = this.prop.a * Math.random();
        
        // sub class's this.prop still initialized, per the sub class' definition,
        // before constructor finishes, so strictPropertyInitialization should be happy.
        this.prop = { a: this.prop.a, b: 2 };
    }
}

@Jessidhia
Copy link

It would still run into the problem, under a strict implementation of stage-3 props, that your declaration of props causes it to be defined anew with a value of undefined.

Try initializing it with = super.props.

@ethanresnick
Copy link
Contributor Author

It would still run into the problem, under a strict implementation of stage-3 props, that your declaration of props causes it to be defined anew with a value of undefined.

I was gonna ask about that. I wanna read the spec closer, but I'm pretty sure super.prop isn't valid syntax (I think it would look for a property/method on the super class's prototype, not on the instance). If that's the case, I imagine there's either a way to do it with this.prop, or perhaps Typescript should not emit a field for fields that have no initializers (i.e., are just there to annotate types).

@RyanCavanaugh RyanCavanaugh added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Dec 15, 2021
@RyanCavanaugh
Copy link
Member

I think the error correctly speaks for itself. { ...this.prop } "should be sugar" for { a: this.prop.a, b: this.prop.b } as much as possible, and it's observable that no sufficient intialization of that form has occurred yet. In this case it all works out in the end, but we're not equipped to do the sort of counterfactual reasoning required to determine that the program is actually OK.

@ethanresnick
Copy link
Contributor Author

@RyanCavanaugh Why does { ...this.prop } have to be like sugar for { a: this.prop.a, b: this.prop.b }? Under the rule I proposed above, it would instead be sugar for { a: this.prop.a } (based on the parent member's type), because the subclass hasn't written to this.prop yet:

Because the compiler is tracking when this.prop is first assigned to in the subclass' constructor, it should be able to tell which reads of this.prop are before that first assignment. For all such reads, it would use the parent class' type for this.prop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants