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

Impossible to use type inference while redefining an inherited class field while reusing the inherited value. #42372

Closed
b6i6o6 opened this issue Jan 16, 2021 · 7 comments
Labels
Duplicate An existing issue was already created

Comments

@b6i6o6
Copy link

b6i6o6 commented Jan 16, 2021

Bug Report

🔎 Search Terms

redefine class fields inference implicit any circular this

🕗 Version & Regression Information

This is the behavior in every version I tried including nightly and I assume it is a consequence of how Typescript currently handle class fields, which is different from the actual implementation of public class fields in current browser engines.

I assume this bug will disappear when Typescript handle class fields as is without transforming them into assignment on this.

⏯ Playground Link

Playground link with relevant code

💻 Code

class Base {
    prop = { a: 42 };
}

class SubWithCircularThis extends Base {
    // This throws: 'prop' implicitly has type 'any' because it does not have a type
    // annotation and is referenced directly or indirectly in its own initializer.(7022)
    // The error would be correct in a standard assignment scenario, but it seems abusive
    // here considering the type of `this.prop` at the moment of the spread operator is
    // actually the type (wrongly) inferred for `super.prop`, see below, thus not circular.
    prop = {
        ...this.prop,
        b: 48,
    }
}

class SubWithoutCircularThisButWithBogusSuperType extends Base {
    // Here `prop` has the correct inferred typing but it is in fact wrong as super
    // references the prototype of Base at this point, which does not have `prop` at
    // all. See the console.log output below.
    prop = {
        ...super.prop,
        b: 48,
    }
}

var bogus = new SubWithoutCircularThisButWithBogusSuperType();
console.log(bogus.prop.a); // output: undefined instead of 42.

🙁 Actual behavior

It is not possible to use type inference when redefining a class field in a sub class using the same name as the super class and reuse the value of the super class field without explicitely typing the proeprty.

It throws: 'prop' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.(7022).

The error would be correct in a standard assignment scenario, but it seems abusive in class fields considering the type of this.prop at the moment of the spread operator is actually the type (wrongly) inferred for super.prop, thus not circular.

However, the type of super.prop, while being the one I would have expected to get for this.prop in the context of class fields definition, is wrong for super in itself as super is the prototype of the Base class, not the instance itself.

So there are two bugs here but they are somewhat linked together because this.prop in the context of class field definition should be the type of super.prop and super.prop should not exist per se.

🙂 Expected behavior

this.prop in the context of class field definition should be the type currently used for super.prop and super.prop should not exist per se as this would more correctly match the behavior of public class fields.

As I mentionned earlier, I assume this would change when Typescript outputs actual public class fields, which is not the case for now, but the behavior seems wrong anyway. Especially the typing of super.prop. Anyway, the compiler should be able to distinguish between the "normal" context, where this.prop indeed means this.prop and the typing would indeed be circular, and the class field context, where this.prop means the prop previously defined in the super class, thus no circularity.

For anyone who would be facing the same issue as I was and who really wants to use type inference rather than manually type the property, I have found a rather hacky workaround to make super.prop return the value of this.prop:

Playground link with hacky workaround

Please do not fix the type of super.prop without fixing the circular type inference of this.prop in the context of class fields, as this would break the workaround. Ideally, these two bugs should be fixed together.

I don't expect this to be fixed before Typescript actually starts outputing javascript code with public class fields but I thought you would like to know nonetheless.

@jcalz
Copy link
Contributor

jcalz commented Jan 16, 2021

Related to #10570

@MartinJohns
Copy link
Contributor

Looks like a duplicate of #33899.

@b6i6o6
Copy link
Author

b6i6o6 commented Jan 16, 2021

Respectfully, I do not think this issue is a duplicate of those two. The first one does not seem to reuse the previous value of the property, thus only encompassing the "redefinition" part of this issue, while the second one uses Object.assign which does not "merge" the types (result of Object.assign(A, B) is A & B) like the spread operator does.

I am not that much into Typescript so I might be wrong and my issue might indeed be a duplicate of those but I don't think any of them include the typing issue of super.prop. This is quite confusing in my opinion because you can only spot the error at runtime when the object is not initialized with the correct value since super.prop is actually undefined in reality.

EDIT: Woops, sorry jcalz I did not realize you said "related to" and not "duplicate". Indeed the two issues are definitely related.

@whzx5byb
Copy link

The issue of super.prop is a bug.

@MartinJohns
Copy link
Contributor

Respectfully, I do not think this issue is a duplicate of those two. [...] while the second one uses Object.assign which does not "merge" the types (result of Object.assign(A, B) is A & B) like the spread operator does.

The behavior is the same when you use object spread: Playground link

But in the end it's in the discretion of the TypeScript team, of course.

EDIT: Woops, sorry jcalz I did not realize you said "related to" and not "duplicate". Indeed the two issues are definitely related.

jcalz said related, I said duplicate. :-)

@RyanCavanaugh
Copy link
Member

I don't think there's anything here not covered by the existing linked issues.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jan 20, 2021
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants