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

Class parameter incorrectly treated as bivariant #56225

Open
ssalbdivad opened this issue Oct 26, 2023 · 9 comments
Open

Class parameter incorrectly treated as bivariant #56225

ssalbdivad opened this issue Oct 26, 2023 · 9 comments
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@ssalbdivad
Copy link

πŸ”Ž Search Terms

class parameter generic bivariant constructor typeof extends

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.3.0-dev.20231026#code/C4TwDgpgBAggRgZ2AJwIYGNirgGwgYQHsA7JZAV00OQB4BLUrY9aAXigG8BfAPinexkMwKMQgB3KAAoAsACgAkADoVqZAHMEALlEQAbhGQBtALryAlPz4MkqZhHnzQkKAElGdlgHkAZjXQkZJTA1Hys8goBjBRUyFAQAB7AEMQAJgiwiCjC2HhE0cHU9MQ+hlA2TCx8APzlHvZQOmIGyI5y6DioCBkAYoSEnBGpEB1q0HBqOijkDnJcbaPdUAAKyIQhNGBrIfFJKemZQpi5BIHTsfyH2ce4pwWxfBxDI53I0BWeEDrutva+m9tCDx5PM5PIAPTgqBeADWTnA0Bgl1W60IRgARB97OizHJIVACQA9aptfGw+EuABCyMBNGcEEIPigfSBGKxLBxEKhRJJYLxUPJcnpUHwlyRiWSaQy1Nq02gOh8qBwCFm+J5pKhABUABZ0DIIbWEcg4VJQODQRXK2bCgBaNNRu0lBxRG3pjOZ-RqUDljSglpVXIJUGJQA

πŸ’» Code

type AbstractableConstructor<instance = {}> = abstract new (
	...args: never[]
) => instance

type InstanceOf<constructor> =
	constructor extends AbstractableConstructor<infer instance> ? instance : never

class Foo {
	declare bar: true
}

class Proto<proto extends AbstractableConstructor = AbstractableConstructor> {
	declare instance: InstanceOf<proto>
}

// Ok
type A = Proto["instance"]
//   ^? {}

// Ok
type B = Proto<typeof Foo>["instance"]
//   ^? Foo

// Ok
type C = A extends B ? true : false
//   ^? false

// This should be false
type Z = Proto extends Proto<typeof Foo> ? true : false
//   ^? true

πŸ™ Actual behavior

proto is treated as bivariant with no structural comparison, resulting in Z evaluating to true.

πŸ™‚ Expected behavior

proto should be treated as covariant as only its return type is used, or compared structurally, resulting in Z evaluating to false.

Additional information about the issue

No response

@RyanCavanaugh
Copy link
Member

Pretty weird. Thankfully easy to work around by putting an out annotation on proto

@ahejlsberg
Copy link
Member

ahejlsberg commented Oct 26, 2023

The issue here is that variance computation classifies the proto type parameter as independent (i.e. unwitnessed, which is true for all types except those that extend AbstractableConstructor). Not clear that we can do any better here, after all the only use of proto is as the source of an inference in InstanceOf and we're simply not able to measure variance for that conditional type.

@ssalbdivad
Copy link
Author

Like @RyanCavanaugh said, the workaround is trivial once you know that this is happening, but getting to that point? Probably much less so in a real-world scenario.

@Andarist mentioned that it might be related to the variance being "unmeasurable." If it can't be inferred accurately, I'd rather have to explicitly cast if I want to allow an assignment like this than get a false positive when I don't.

Is there any way in this situation to treat the parameter as invariant rather than bivariant without breaking an unacceptable amount of existing code?

@ahejlsberg
Copy link
Member

We could potentially consider some sort of diagnostic when variance is measured as independent as that is generally not intended. However, that no doubt would be a breaking change and would need to be under (yet another) compiler switch.

Alternatively this could be a lint thing, provided the necessary type checker information is exposed in our API.

@ahejlsberg
Copy link
Member

Hmm, actually, we measure InstanceOf to be bivariant and therefore Proto also ends up measuring bivariant. The bivariance comes from effects of the rule:

// Two conditional types 'T1 extends U1 ? X1 : Y1' and 'T2 extends U2 ? X2 : Y2' are related if
// one of T1 and T2 is related to the other, U1 and U2 are identical types, X1 is related to X2,
// and Y1 is related to Y2.

It does seem a bit arbitrary to pick bivariance as we don't end up measuring any differences in outcomes between the marker types we use for variance probing. Effectively, we're saying that when we can't measure variance for a conditional type, we regard that conditional type to be bivariant.

@whzx5byb
Copy link

@ahejlsberg

InstanceOf is treated as covariant correctly here.

type T1 = InstanceOf<AbstractableConstructor> extends InstanceOf<typeof Foo> ? true : false;
//   ^? type T1 = false
type T2 = InstanceOf<typeof Foo> extends InstanceOf<AbstractableConstructor> ? true : false;
//   ^? type T2 = true

@ahejlsberg
Copy link
Member

InstanceOf is treated as covariant correctly here.

Right, what's going on here is that InstanceOf<AbstractableConstructor> and InstanceOf<typeof Foo> are resolved to {} and Foo respectably before the conditional type is evaluated. So, it's like writing {} extends Foo ? true : false, and at that point we don't even consult the variance computed for InstanceOf.

But in the case of relating two instances of Proto we do consult the variance information, and, since it claims bivariance, we consider the Proto<AbstractableConstructor> and Proto<typeof Foo> to be related in both directions. You can see the difference by forcing a structural comparison:

class Proto2<proto extends AbstractableConstructor = AbstractableConstructor> {
	declare instance: InstanceOf<proto>
}

type Z2 = Proto extends Proto2<typeof Foo> ? true : false
//   ^? false

Ideally we'd detect when conditional type variance is unmeasurable and instead force structural comparisons, but we've tried that in the past with pretty disastrous results for performance.

@ethanresnick
Copy link
Contributor

I found this a bit hard to understand, so put together a simpler repro. Figured I'd post it here in case it helps someone else:

type Id<T = {}> = T

type ConditionalId<T> = T extends Id<infer Value> ? Value : never

class Box<B> {
  declare value: ConditionalId<B>
}

// Ok
type A = Box<{}>["value"]
//   ^? {}

// Ok
type B = Box<{ foo: true }>["value"]
//   ^? { foo: true }

// Ok
type C = A extends B ? true : false
//   ^? false

// This should be false
type Z = Box<{}> extends Box<{ foo: true }> ? true : false
//   ^? true

@rotu
Copy link

rotu commented Feb 22, 2024

Another workaround, without manual variance annotations, is to extract the inline type expression into a new type variable.

// change this:
class Proto<proto extends AbstractableConstructor = AbstractableConstructor> {
  declare instance: InstanceOf<proto>
}

// to this:
class Proto<proto extends AbstractableConstructor = AbstractableConstructor, T = InstanceOf<proto>> {
  declare instance: T
}

or, in the reduced example:

// change this
class Box<B> {
  declare value: ConditionalId<B>
}

// to this
class Box<B, T = ConditionalId<B>> {
  declare value: T
}

That is, the type is conditionally independent of proto/B, given T. This seems much cheaper than structural comparison and pretty straightforward to automate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

6 participants