-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Generic Extended Types not inferring Correctly #48741
Comments
This is a very common misconception. Your generic type is extending For example: interface MyType {
prop1: (5 | 6)[]
prop2: ("a" | "b")[]
}
new SubKlass<MyType>() Assigning |
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 class SubKlass<T extends DefaultProps = DefaultProps> extends SuperKlass<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. |
This is why: interface ExtendedProps2 extends DefaultProps {
prop1: (2 | 3)[];
}
class OneDeeper2 extends SubKlass<ExtendedProps2> {
method2(){
this.unwrapped.prop1 = 1; // correct error
}
} Since In short: The compiler is enforcing that |
@fatcerberus That part totally makes sense to me. The part that is broken isn't within the "OneDeeper" class, its within the "SubKlass" class. class SubKlass<T extends DefaultProps = DefaultProps> extends SuperKlass<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 interface ExtendedProps2 extends DefaultProps {
prop1: (2 | 3)[];
}
const foo = new SubKlass<ExtendedProps2>(); // legal instantiation
foo.method1(); // oops! foo.unwrapped.prop1 is now 1 but this is not legal for a SubKlass<ExtendedProps2> The error prevents you from making this mistake. |
@fatcerberus What do you think should happen if I call method1 in SubKlass that looks like this. class SubKlass<T extends DefaultProps = DefaultProps> extends SuperKlass<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. class SubKlass<T extends DefaultProps = DefaultProps> extends SuperKlass<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. |
This is not strictly true, |
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 |
@whzx5byb If you look at the code in the playground you will see the error, that to me, seems broken. It reads, class SubKlass<T extends DefaultProps = DefaultProps> extends SuperKlass<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. |
Here is a more concise example of what I believe to be a bug. type UnWrapValue<T> = T extends Array<infer TValue> ? TValue | null : never;
type UnWrapObject<T> = {
[P in keyof T]: UnWrapValue<T[P]>;
};
interface DefaultProps {
prop1: number[];
prop2: string[];
}
interface P extends DefaultProps {
prop3: boolean[];
}
// Unwrapping P here works as expected
const a: UnWrapObject<P> = {
prop1: 1,
prop2: "",
prop3: false
};
class Car<P extends DefaultProps> {
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. The error you're seeing is not a bug, unless you believe this behavior should be allowed: interface Fooey extends DefaultProps { prop1: (40 | 41 | 42)[] }
const foo = new SubKlass<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: class SubKlass<T extends DefaultProps = DefaultProps> extends SuperKlass<T> {
method1(){
this.original.prop1 = [ 1, 2, 3 ];
}
} 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. |
@fatcerberus |
I assumed a narrowing of "T" as developers inherited "SubKlass", but this doesn't HAVE to happen as @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. interface Props {
prop1: string[];
prop2: number[];
};
interface NarrowedProps {
prop1: ["Foo", "Bar"],
prop2: [1, 2]
}
class Klass <T extends Props> {
props!: T;
method(){
this.props.prop2.push(3);
}
}
class SubKlass extends Klass<NarrowedProps> {
override method(){
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.
const c = new Klass<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 |
@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. |
@RyanCavanaugh This best explains what I see as a bug. With a playground. |
Gotcha. I don't have anything to add over @fatcerberus 's answer re: that one; it's the correct behavior. |
@RyanCavanaugh Thanks guys |
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: type Foo<T> = T extends infer U ? U : never;
function fn<T extends number>(x: Foo<T>, s: number) {
var v1: Foo<number> = 0 as any;
// ^? v1: number
// The result of Foo<number> is evaluated as number.
var v2: Foo<T> = 0 as any;
// ^? 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,
I think this could answer the question you raised in origin post.
Because
Because |
@whzx5byb What a beautiful reply. That was really helpful, thank you :). |
Bug Report
🔎 Search Terms
🕗 Version & Regression Information
All versions have this problem.
⏯ Playground Link
Playground link with relevant code
💻 Code
🙁 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".
The text was updated successfully, but these errors were encountered: