Skip to content

Regression: Covariance fails in latest TS release? #29590

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

Closed
nicojs opened this issue Jan 25, 2019 · 9 comments
Closed

Regression: Covariance fails in latest TS release? #29590

nicojs opened this issue Jan 25, 2019 · 9 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@nicojs
Copy link

nicojs commented Jan 25, 2019

TypeScript Version: 3.3.0-dev.20190125

Search Terms:
covariance

Code

type Injector<TContext> = {
  resolve: <T extends keyof TContext>() => T extends keyof TContext ? TContext[T] : never;
}

const foo: Injector<{ foo: string, bar: string }> = {} as any;
const bar: Injector<{ bar: string }> = foo; // no error
console.log(bar);

Expected behavior:
No compiler error with --strictFunctionTypes. As was the case in TS 3.2.2

Actual behavior:
Compiler error (since TS 3.2.4)

index.ts:18:7 - error TS2322: Type 'Injector<{ foo: string; bar: string; }>' is not assignable to type 'Injector<{ bar: string; }>'.
  Property 'foo' is missing in type '{ bar: string; }' but required in type '{ foo: string; bar: string; }'.

18 const bar: Injector<{ bar: string }> = foo;
         ~~~

  index.ts:17:23
    17 const foo: Injector<{ foo: string, bar: string }> = {} as any;
                             ~~~
    'foo' is declared here.

Playground Link:
https://www.typescriptlang.org/play/index.html#src=type%20Injector%3CTContext%3E%20%3D%20%7B%0D%0A%20%20resolve%3A%20%3CT%20extends%20keyof%20TContext%3E()%20%3D%3E%20T%20extends%20keyof%20TContext%20%3F%20TContext%5BT%5D%20%3A%20never%3B%0D%0A%7D%0D%0A%0D%0Aconst%20foo%3A%20Injector%3C%7B%20foo%3A%20string%2C%20bar%3A%20string%20%7D%3E%20%3D%20%7B%7D%20as%20any%3B%0D%0Aconst%20bar%3A%20Injector%3C%7B%20bar%3A%20string%20%7D%3E%20%3D%20foo%3B%20%2F%2F%20no%20error%0D%0Aconsole.log(bar)%3B

Related Issues:

(maybe): #28752

@nicojs
Copy link
Author

nicojs commented Jan 26, 2019

It might work as intended, but I'm not sure why Injector<TContext> wouldn't be covariant in TContext. If anyone can point that out to me, I would be very grateful.

You can also look at typed-inject. I've added a test for covariance. Running it with ts 3.2.2 works fine, 3.2.4 (or 3.3.0-rc) gives the error.

@fatcerberus
Copy link

I think, but not positive, that type parameters are (supposed to be) completely invariant in TS. There is an open proposal to add co- and contravariance annotations a la C#, see #10717, but AFAICT nothing has been implemented yet.

@nicojs
Copy link
Author

nicojs commented Jan 29, 2019

I don't think that's true. See this comment by @ahejlsberg #10717 (comment)

@jack-williams
Copy link
Collaborator

jack-williams commented Jan 29, 2019

Smaller repro?

interface Injector<TContext> {
  resolve: TContext[keyof TContext];
}

const foo: Injector<{ foo: string, bar: string }> = {} as any;
const bar: Injector<{ bar: string }> = foo;
console.log(bar);

@nicojs
Copy link
Author

nicojs commented Jan 30, 2019

@jack-williams I don't think that's a reproduction of my issue. Your code example already gave a compile error in [email protected] (just tested it). However, it might indicate that it my example also should have been a compile error in [email protected].

@jack-williams
Copy link
Collaborator

jack-williams commented Jan 30, 2019

Ooops, sorry --- got carried away deleting code! What about this?

interface Injector<TContext> {
  resolve: <T extends keyof TContext>() => T extends keyof TContext ? TContext[T] : never;
}

const foo: Injector<{ foo: string, bar: string }> = {} as any;
const bar: Injector<{ bar: string }> = foo;
console.log(bar);

There have been a few recent changes to conditional types; it may the be the case that one of those have affected this.

Seems to work for type aliases, but not interfaces:

type Injector<TContext> = {
  resolve: <T extends keyof TContext>() => T extends keyof TContext ? TContext[T] : never;
}

const foo: Injector<{ foo: string, bar: string }> = {} as any;
const bar: Injector<{ bar: string }> = foo; // no error
console.log(bar);

@nicojs
Copy link
Author

nicojs commented Jan 30, 2019

@jack-williams
exactly! I'll update my original post. Thanks.

Seems to work for type aliases, but not interfaces:

Do you think this was an unintended side effect? Or is there a reason for this change?

@ahejlsberg
Copy link
Member

This is basically a design limitation.

In --strictFunctionTypes mode we compute a variance digest for type parameters of classes and interfaces up front and use that when relating instantiations, whereas we always perform a structural comparison for object type literals (see #18654 for more discussion of the details). Our variance digest is sometimes too conservative and that's causing the issue here. Specifically, given a type parameter T and a type parameter U extends T, T[keyof T] is neither a supertype nor a subtype of U[keyof U]. Therefore a class or interface that uses this construct on a particular type parameter will be considered invariant in that type parameter. This misses the subtlety that T[keyof T] and U[keyof U] are sometimes covariant, e.g. when extra properties in U all have types that already exist as types of properties in T. The only way to "see" that is to always perform a full structural comparison, but we don't want to do that for classes and interfaces as the performance regressions are rather draconian.

@ahejlsberg ahejlsberg added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jan 30, 2019
@nicojs
Copy link
Author

nicojs commented Jan 31, 2019

Ok. I'm assuming that the fact that it didn't report errors in 3.2.2 was a bug, and it now works as intended. Closing because it is a design limitation.

Given a type parameter T and a type parameter U extends T, T[keyof T] is neither a supertype nor a subtype of U[keyof U].

Got it, thanks for replying!

@nicojs nicojs closed this as completed Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants