You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Generic Extended Types not inferring Correctly #48741
typeUnWrapValue<T>=TextendsArray<infer TValue> ? TValue|null : never;typeUnWrapObject<T>={[PinkeyofT]: UnWrapValue<T[P]>;};interfaceDefaultProps{prop1: number[];prop2: string[];}interfaceExtendedPropsextendsDefaultProps{prop3: boolean[];}classSuperKlass<T>{unwrapped!: UnWrapObject<T>;original!: T;}// It appears to me that the default generic type isn't being accounted for here.// I would expect T to be at least DefaultProps and and be able to get that typing, however, it // is unable to infer the typing.classSubKlass<TextendsDefaultProps=DefaultProps>extendsSuperKlass<T>{method1(){// Why does the typing NOT work here?this.unwrapped.prop1=1;}}classOneDeeperextendsSubKlass<ExtendedProps>{method2(){// Why does the typing work here?this.unwrapped.prop1=1;this.unwrapped.prop3=false;}}
🙁 Actual behavior
Unwrapping a constrained generic type with "infer" fails to do so properly.
🙂 Expected behavior
"this.unwrapped.prop1" should be type "string" within "SubKlass".
I'm not sure that that answers the question. If you look at the example in the playground and inside the SubKlass class you write
classSubKlass<TextendsDefaultProps=DefaultProps>extendsSuperKlass<T>{method1(){// Why does the typing not work here?this.unwrapped.prop1=1;this.original.prop1;// This line correctly says that "prop1" is a "string[]"}}
So the question is why doesn't the unwrapping work, and correctly give me the inferred type of that array.
I'm probably missing something, or I'm not explaining myself well.... probably both.
Since SubKlass<ExtendedProps2> is a valid instantiation of SubKlass, the assignment inside SubKlass#method1 is not sound and the compiler correctly flags it as an error. There's no error inside OneDeeper because, in that case, SubKlass has already been instantiated as a concrete type and prop1 is known be number | null.
In short: The compiler is enforcing that SubKlass's implementation works correctly for any type of T.
Bruce Pascoe (@fatcerberus) That part totally makes sense to me. The part that is broken isn't within the "OneDeeper" class, its within the "SubKlass" class.
classSubKlass<TextendsDefaultProps=DefaultProps>extendsSuperKlass<T>{method1(){this.unwrapped.prop1=1;// This does NOT work as expected. This should have been type number, but it doesn't infer that.this.original.prop1.push(1);// This correctly infers from T that is has to be at least an array of numbers.}}``
And that's what I'm trying to tell you - the error in SubKlass is expected:
interfaceExtendedProps2extendsDefaultProps{prop1: (2|3)[];}constfoo=newSubKlass<ExtendedProps2>();// legal instantiationfoo.method1();// oops! foo.unwrapped.prop1 is now 1 but this is not legal for a SubKlass<ExtendedProps2>
Bruce Pascoe (@fatcerberus)
But the error doesn't prevent this on the "original" property of the "SubKlass". Let me explain.
What do you think should happen if I call method1 in SubKlass that looks like this.
classSubKlass<TextendsDefaultProps=DefaultProps>extendsSuperKlass<T>{method1(){// Should this allow me to push into the array of numbers with any number?this.original.prop1.push(1);}}
In this case it works as I would expect. Because T extends DefaultProps, it knows that type "T" MUST have a property of "prop1" of type "number[]" based on the constraint provided to the generic.
Now lets modify the method1 to look like this.
classSubKlass<TextendsDefaultProps=DefaultProps>extendsSuperKlass<T>{method1(){// I expected this to be type "number" within SubKlass. Why isn't it? It had the same constraints on T. // Why does T not send its constraints to UnWrapObject<T>.this.unwrapped.prop1=1;}}
I'm really trying to understand, It doesn't make sense to me that inside of the "SubKlass" class the "original" property knows that T has to be at least a "number[]" because it extends "DefaultProps", but when I send T to be unwrapped through UnWrapObject it forgets that T extends "DefaultProps", and therefore has two properties with specific types.
Don't give up on explaining. I want to understand why T at times acts as expected within "SubKlass", but then, when I infer what the array types are it seems to forget that T extends DefaultProps.
In this case it works as I would expect. Because T extends DefaultProps, it knows that type "T" MUST have a property of "prop1" of type "number[]" based on the constraint provided to the generic.
This is not strictly true, T may have a property prop1 of type (e.g.) (2 | 3)[] as I illustrated above. The push call should be an error for the same reason; I'm not sure why it isn't.
method1(){
// Should this allow me to push into the array of numbers with any number?
this.original.prop1.push(1);
}
This is a common unsoundness caused by mutability. To not break type soundness, mutable properties, and mutable arrays, must be invariant. However in ts they are covariant, that's why this.original.prop1.push(1) is allowed.
whzx5byb
So with "this.original.prop1.push(1)" being covariant, wouldn't that be the same for "this.unwrapped.prop1". I suppose this is what seems wrong to me. If "this.original.prop1" is covariant then shouldn't "this.unwrapped.prop1" be too.
If you look at the code in the playground you will see the error, that to me, seems broken. It reads, Type '1' is not assignable to type 'UnWrapValue<T["prop1"]>'.(2322) It's acting like it has no clue that type "T" extends "DefaultProps".
classSubKlass<TextendsDefaultProps=DefaultProps>extendsSuperKlass<T>{method1(){this.original.prop1.push(1);this.unwrapped.prop1=1;// <-- This is where the bug is to me. }}
For me it doesn't make sense to have "T extends DefaultProps" be anything but covariant. I don't see how contravariant would be helpful here, nor invariant. The keyword extends implies covariant, but I could be missing something.
typeUnWrapValue<T>=TextendsArray<infer TValue> ? TValue|null : never;typeUnWrapObject<T>={[PinkeyofT]: UnWrapValue<T[P]>;};interfaceDefaultProps{prop1: number[];prop2: string[];}interfacePextendsDefaultProps{prop3: boolean[];}// Unwrapping P here works as expected consta: UnWrapObject<P>={prop1: 1,prop2: "",prop3: false};classCar<PextendsDefaultProps>{unwrapped!: UnWrapObject<P>// Unwrapping here is broken.method(){this.unwrapped.prop1=1;// <--- This is the bug and should be type number.this.unwrapped.prop2="";// <--- This is the bug and should be type string.}}
Variance kind of clouds the issue here. T extends DefaultProps means that T is a subtype of DefaultProps. So yes, that part is covariant. However, for writes to a generic type to be sound, they must be contravariant. This severely limits what you can safely hard-code into a generic function or class.
The error you're seeing is not a bug, unless you believe this behavior should be allowed:
interfaceFooeyextendsDefaultProps{prop1: (40|41|42)[]}constfoo=newSubKlass<Fooey>();foo.method1();console.log(foo.unwrapped.prop1);// prints 1, even though this should be impossible given the types involved
I don't know how much clearer I can make this. It's true that this is allowed by the compiler:
But that's not safe for the exact same reason and should be an error. However, perfect soundness is not a goal in the design of the type system, so this case probably just got overlooked.
I assumed a narrowing of "T" as developers inherited "SubKlass", but this doesn't HAVE to happen as Bruce Pascoe (@fatcerberus) explained so well above. Developers could just narrow "T" and break the typing within the "SubKlass". This is allowed, probably, to be practical.
Here is what I assumed in psuedo code.
interfaceProps{prop1: string[];prop2: number[];};interfaceNarrowedProps{prop1: ["Foo","Bar"],prop2: [1,2]}classKlass<TextendsProps>{props!: T;method(){this.props.prop2.push(3);}}classSubKlassextendsKlass<NarrowedProps>{overridemethod(){this.props.prop2[0]=1}}// To this point, this all works well, however, the type system would also allow this.// And in this case the type system doesn't protect you from runtime errors. constc=newKlass<NarrowedProps>();
This all makes sense that the type system has to be practical. What I would like to see, is that the unwrapping of "T" when extending Props would infer the correct types. This is where I believe the inconsistency is difficult to deal with. I gave an example above that showed unwrapping being inconsistent when dealing with interface vs classes. That is my true concern.
What is UnWrapObject<T>? It could be { prop1: number }, or it could be { prop1: 1 | 2 | 3 }. We don't know for sure until we also know what T is, since as already established, existing properties can be narrowed by subtypes. I think that's contributing to the confusion here. In the case of original, TS is just (unsoundly) using the constraint of T (for convenience, most likely), but it won't do that when feeding T through another type alias.
Bruce Pascoe (@fatcerberus) I see, well said again. So it was a compromise they made with "T" within "SubKlass", but did not make the same compromise when using "infer" within the UnWrapValue. I think I understand now. How do we find out if this was intentional or not?
Generally if something has been the behavior for multiple years, it's intentional. Finding years-old bugs in plain view in widely-used software is quite rare.
This thread is pretty long, if you want to construct a short example that demonstrates the question I'd be happy to speak to it.
I believe the essential problem is some mechanisms called 'Assignability to Conditional Types'. This logic is described in #46429, and #30639 (an earlier implementation before v4.5).
Let's begin with a simple code:
typeFoo<T>=Textends infer U ? U : never;functionfn<Textendsnumber>(x: Foo<T>,s: number){varv1: Foo<number>=0asany;// ^? v1: number// The result of Foo<number> is evaluated as number.varv2: Foo<T>=0asany;// ^? v2: Foo<T>// The result of Foo<T> is lazy-evaluating.v1.toExponential();// Compiles. v1 is type number.v2.toExponential();// Compiles. v2 is treated as type number.v1=1;// Compiles. v2=1;// Error. Type 'number' is not assignable to type 'Foo<T>'.}
As #46429 says, Type 'number' is not assignable to type 'Foo<T>' because:
The conditional type is distributive, and A (T) is referenced in C (T).
B (infer U) has infer positions.
S (number) is not assignable to C (T).
S (number) is not assignable to D (never).
I think this could answer the question you raised in origin post.
Because T is not evaluated, so this.unwrapped.prop1 is a lazy-evaluating conditional type UnWrapValue<T>. You can't assign number to it, as I explained above.
Because T is evaluated as ExtendedProps, and this.unwrapped.prop1 is no longer an unresolved conditional type. Its value is evaluated as number | null. That's why you can assign number to it.
Activity
MartinJohns commentedon Apr 17, 2022
This is a very common misconception. Your generic type is extending
DefaultProps, but it is not aDefaultProps. The generic type can be more specific.For example:
Assigning
1toprop1would be invalid, as the type only accepts5or6.jaredjbarnes commentedon Apr 17, 2022
I'm not sure that that answers the question. If you look at the example in the playground and inside the SubKlass class you write
So the question is why doesn't the unwrapping work, and correctly give me the inferred type of that array.
I'm probably missing something, or I'm not explaining myself well.... probably both.
fatcerberus commentedon Apr 18, 2022
This is why:
Since
SubKlass<ExtendedProps2>is a valid instantiation ofSubKlass, the assignment insideSubKlass#method1is not sound and the compiler correctly flags it as an error. There's no error insideOneDeeperbecause, in that case,SubKlasshas already been instantiated as a concrete type andprop1is known benumber | null.In short: The compiler is enforcing that
SubKlass's implementation works correctly for any type ofT.jaredjbarnes commentedon Apr 18, 2022
Bruce Pascoe (@fatcerberus) That part totally makes sense to me. The part that is broken isn't within the "OneDeeper" class, its within the "SubKlass" class.
fatcerberus commentedon Apr 18, 2022
And that's what I'm trying to tell you - the error in
SubKlassis expected:The error prevents you from making this mistake.
jaredjbarnes commentedon Apr 18, 2022
Bruce Pascoe (@fatcerberus)
But the error doesn't prevent this on the "original" property of the "SubKlass". Let me explain.
What do you think should happen if I call method1 in SubKlass that looks like this.
In this case it works as I would expect. Because T extends DefaultProps, it knows that type "T" MUST have a property of "prop1" of type "number[]" based on the constraint provided to the generic.
Now lets modify the method1 to look like this.
I'm really trying to understand, It doesn't make sense to me that inside of the "SubKlass" class the "original" property knows that T has to be at least a "number[]" because it extends "DefaultProps", but when I send T to be unwrapped through UnWrapObject it forgets that T extends "DefaultProps", and therefore has two properties with specific types.
Don't give up on explaining. I want to understand why T at times acts as expected within "SubKlass", but then, when I infer what the array types are it seems to forget that T extends DefaultProps.
fatcerberus commentedon Apr 18, 2022
This is not strictly true,
Tmay have a propertyprop1of type (e.g.)(2 | 3)[]as I illustrated above. Thepushcall should be an error for the same reason; I'm not sure why it isn't.whzx5byb commentedon Apr 18, 2022
Jared Barnes (@jaredjbarnes)
This is a common unsoundness caused by mutability. To not break type soundness, mutable properties, and mutable arrays, must be invariant. However in ts they are covariant, that's why
this.original.prop1.push(1)is allowed.See a more detailed answer in SO
jaredjbarnes commentedon Apr 18, 2022
whzx5byb
So with "this.original.prop1.push(1)" being covariant, wouldn't that be the same for "this.unwrapped.prop1". I suppose this is what seems wrong to me. If "this.original.prop1" is covariant then shouldn't "this.unwrapped.prop1" be too.
If you look at the code in the playground you will see the error, that to me, seems broken. It reads,
Type '1' is not assignable to type 'UnWrapValue<T["prop1"]>'.(2322)It's acting like it has no clue that type "T" extends "DefaultProps".For me it doesn't make sense to have "T extends DefaultProps" be anything but covariant. I don't see how contravariant would be helpful here, nor invariant. The keyword extends implies covariant, but I could be missing something.
jaredjbarnes commentedon Apr 18, 2022
Here is a more concise example of what I believe to be a bug.
Playground example
fatcerberus commentedon Apr 18, 2022
Variance kind of clouds the issue here.
T extends DefaultPropsmeans thatTis a subtype ofDefaultProps. So yes, that part is covariant. However, for writes to a generic type to be sound, they must be contravariant. This severely limits what you can safely hard-code into a generic function or class.The error you're seeing is not a bug, unless you believe this behavior should be allowed:
I don't know how much clearer I can make this. It's true that this is allowed by the compiler:
But that's not safe for the exact same reason and should be an error. However, perfect soundness is not a goal in the design of the type system, so this case probably just got overlooked.
jaredjbarnes commentedon Apr 18, 2022
Bruce Pascoe (@fatcerberus)
Well said. :)
jaredjbarnes commentedon Apr 18, 2022
I assumed a narrowing of "T" as developers inherited "SubKlass", but this doesn't HAVE to happen as Bruce Pascoe (@fatcerberus) explained so well above. Developers could just narrow "T" and break the typing within the "SubKlass". This is allowed, probably, to be practical.
Here is what I assumed in psuedo code.
This all makes sense that the type system has to be practical. What I would like to see, is that the unwrapping of "T" when extending Props would infer the correct types. This is where I believe the inconsistency is difficult to deal with. I gave an example above that showed unwrapping being inconsistent when dealing with interface vs classes. That is my true concern.
fatcerberus commentedon Apr 18, 2022
What is
UnWrapObject<T>? It could be{ prop1: number }, or it could be{ prop1: 1 | 2 | 3 }. We don't know for sure until we also know whatTis, since as already established, existing properties can be narrowed by subtypes. I think that's contributing to the confusion here. In the case oforiginal, TS is just (unsoundly) using the constraint ofT(for convenience, most likely), but it won't do that when feedingTthrough another type alias.jaredjbarnes commentedon Apr 18, 2022
Bruce Pascoe (@fatcerberus) I see, well said again. So it was a compromise they made with "T" within "SubKlass", but did not make the same compromise when using "infer" within the UnWrapValue. I think I understand now. How do we find out if this was intentional or not?
RyanCavanaugh commentedon Apr 18, 2022
Generally if something has been the behavior for multiple years, it's intentional. Finding years-old bugs in plain view in widely-used software is quite rare.
This thread is pretty long, if you want to construct a short example that demonstrates the question I'd be happy to speak to it.
jaredjbarnes commentedon Apr 18, 2022
Ryan Cavanaugh (@RyanCavanaugh) This best explains what I see as a bug. With a playground.
Best Example
RyanCavanaugh commentedon Apr 18, 2022
Gotcha. I don't have anything to add over Bruce Pascoe (@fatcerberus) 's answer re: that one; it's the correct behavior.
jaredjbarnes commentedon Apr 18, 2022
Ryan Cavanaugh (@RyanCavanaugh) Thanks guys
whzx5byb commentedon Apr 18, 2022
Jared Barnes (@jaredjbarnes)
I believe the essential problem is some mechanisms called 'Assignability to Conditional Types'. This logic is described in #46429, and #30639 (an earlier implementation before v4.5).
Let's begin with a simple code:
As #46429 says,
Type 'number' is not assignable to type 'Foo<T>'because:A (T)is referenced inC (T).B (infer U)has infer positions.S (number)is not assignable toC (T).S (number)is not assignable toD (never).I think this could answer the question you raised in origin post.
Because
Tis not evaluated, sothis.unwrapped.prop1is a lazy-evaluating conditional typeUnWrapValue<T>. You can't assignnumberto it, as I explained above.Because
Tis evaluated asExtendedProps, andthis.unwrapped.prop1is no longer an unresolved conditional type. Its value is evaluated asnumber | null. That's why you can assignnumberto it.jaredjbarnes commentedon Apr 18, 2022
whzx5byb What a beautiful reply. That was really helpful, thank you :).