-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Incorrect variance inference when parameters of an interface with eventually different variances are bundled in an object type #56069
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
Comments
I don't really see a good possible fix here. Variance is computed per type parameter, so The workarounds work because they either cause a structural type relation to occur instead of a variance-based one, or split out the type parameter so that each one has its own variance. |
I don't know how it works internally, I imagine you compute variance during creation and one needs to consider interface merging, but if the variance check is followed by a structural check, then maybe considering |
Falling back to structural comparison when a variance comparison is available is somewhere between a 40% and (never terminates) hit to performance depending on the codebase |
Correct me if I am wrong, but if someone needs to have an interface with arbitrary/dynamic arity that supports variance (I used distinct fields in CreateFoo but it doesn't have to be this way), there is no other way than relying on a structural check. If At current no one uses this pattern because it does not work, so I imagine the only additional overhead for 100% of code bases would be limited to the additional logic required to single out this case. I understand that, if it is a feature request, it is not appealing. The use case that really benefits from it is niche and there is a workaround. |
I think the targeted form of that request would "just" be to add a variance annotation that more or less disables variance-based relation (a thing that can happen organically today anyway): interface Foo<unreliable T> { } // bikeshed name I'm not sure I'd feel comfortable handing such a dangerous weapon to users, though. FWIW we've tried to fix this once already and had to revert it, see #54754 / #54781 and the links from there |
Yes I was hesitant to suggest that. An annotation which sole purpose is to disable an optimisation is too coupled with the internals of the compiler to be added to the language, especially if you are actively trying to solve this problem by other means. We could be stuck with a useless annotation. |
I realise one can do abusive stuff with this type Foo<P extends unknown[]> = CreateFoo<[
// we set arbitrary variances
P[0],
(a: P[1]) => void,
(a: P[2]) => P[2]
]>
interface CreateFoo<P extends any[]> {
// we turn everything back to covariant
covariant: [
P[0],
Parameters<P[1]>[0],
ReturnType<P[2]>
]
// some other field
overrideMe: any
}
type Direct = CovariantCheck<Foo<[number, string, boolean]>, [2, 'a', true]> // OK
type CovariantCheck<T extends { covariant: any }, U extends T['covariant']> = never
type Indirect<T extends Foo<[number, string, boolean]>> =
CovariantCheck<T, [2, 'a', true]>; // Generic OK
type OriginalVarianceCheck = Indirect<Foo<[2, unknown, boolean]>> // OK If it didn't rely on unintended behaviour this would actually be pretty need for implementing HKT because, like functions, you don't check variance the same way when you apply it with arguments and when you pass it to a higher order function. Now this state is actually pretty fragile because overriding any field falls back to the structural check interface Bar extends Foo<[2, unknown, boolean]> {
overrideMe: any
}
type IBar = Indirect<Bar>
// ~~~
// Type 'unknown' is not assignable to type 'string' |
I think the original issue is in error. Object properties are unsuitable for contravariant type parameters. TypeScript has no type FooParams<in X, out Y> = { x: X, y: Y };
// Type 'FooParams<super-X, Y>' is not assignable to type 'FooParams<sub-X, Y>' as implied by variance annotation.
// Types of property 'x' are incompatible.
// Type 'super-X' is not assignable to type 'sub-X'. A good alternative, if you want to keep interface CreateFoo<
P extends FooParams, // type is *independent* of P
PIn = P['in'], // inferred as contravariant
POut = P['out'] // inferred as covariant
> {
in: (contra: PIn) => void
out: () => POut
} |
🔎 Search Terms
"variance", "object type", "interface", "higher order type", "type constraint", "generics"
🕗 Version & Regression Information
This is the behavior in every version I tried, and I reviewed the FAQ for entries about variance
⏯ Playground Link
https://www.typescriptlang.org/play?ts=5.2.2#code/C4TwDgpgBAYg9nAPASQHYBooHkCuwB8UAvFAMIBOEAhsBPEgN5QCWqAXFGpnHh7sFAC++AFAjWtcgDMqAY2gVqteogAKUCAA9aqACYBnWAlVVyVALb7CDEVBbsoACllxUwMx1UBtAOSsfALoAlMSEAG5wzLq2UDzAHI4hRITePnGBIoJioJBGcCZmlsRQTKwcVKgg3LxQFSBCANzZ4NBousyUssAqACoa2hB6hir67qwA5pioOOYARhDk+IQkPU0iOa30xW0dEF0qI2Ook1AAjEsiAPSXdrd39w92AH4vr29vVzc9LVA+h+QTTDnHxQXRwCCGVBwAT6GjMfRSerAAAW0BcqFGZgkv3+gKg0zmC3wPgAdJ87N9cj4CfNyCD4fjobV9PpmONUFRZgAbaDAOBQDa-U4+MRXABUUCkzE0UDFl2auQAoppIF1ED1liUoF4ANL2KAAawgIDgUigPQCHB6uoCQgVrT0u32CBg0ogunV-R0BigytVwEQuOOUxmtKWmtW9s49CwepIO063Rdbo9QZO50I12wOrElwl6PccC5svl6x+ACFTCgMNg8JrFDQIJXyIhSg40EJMPxROI3AsZPIyJRG821F7Bj7mwULPou3WSjEyk4Cx4oKl-MFQlAIlEYnEEklCPxMlGp6YZ8U2+VKo0owm9sBR30tN7DKPMXiaUSI2tBchm9sjqJqO75HOmSxQFmsainmUCwuY0BhKYzAVIOcpRqQcAqFwtYEMUDa0JhKhXpwNb7rhQg9hI-ZyAow6EVhCBji+E7DMY56WNYi4OOuqAZHY5GpOkAQnmWuT3l0RFMc+AxDGQjFIB+wb4qG37FJGYmtFJ-LxkBD7aYgBk+FQPiYAATBBUE6kAA
💻 Code
🙁 Actual behavior
Foo
has become invariant onIn
andOut
(it would not acceptFoo<unknown, number>
either).🙂 Expected behavior
I would expect wrapping parameters in an object to be transparent.
Foo
should remain contravariant onIn
and covariant onOut
Additional information about the issue
It looks like variance information is not propagated correctly specifically when
Expect
is used to convert the interface into an object type, the variance check works properly;CoFoo
is covariant on bothIn
andOut
as expected and aContraFoo
would behave similarly.The text was updated successfully, but these errors were encountered: