Contravariance broke in 3.8 when combining spreading parameter list with conditional type

[nicojs]

TypeScript Version: 3.8.3

Search Terms: variance contravariance

Description: An Injector in typed-inject is a dependency injection container that is type safe. It can only provide values for tokens it knows. A small example:

declare const fooInjector: Injector<{foo: string}>; // this injector can provide string values for the token 'foo'
const foo = fooInjector.resolve('foo'); 
// => typeof foo === 'string'
fooInjector.resolve('bar');
// => Error ts(2345) Argument of type '"bar"' is not assignable to parameter of type '"foo"'

It makes sense that an injector Injector<{}> is not assignable to Injector<{foo: string}>, since it cannot provide a value for token 'foo'. This was the case in TS 3.7. However, since TS 3.8, Injector<{}> is assignable to Injector<{foo: string}> 😪.

declare const rootInjector: Injector<{}>;
const fooInjector: Injector<{ foo: string}> = rootInjector;

Expected behavior: Type 'Injector<{}>' is not assignable to type 'Injector<{ foo: string; }>'.

Actual behavior: No error

Related Issues: Couldn't find any 🤷‍♂️

Code

I think I've trimmed it down to the essentials.

interface Injector<TContext> {
  injectFunction<Tokens extends (keyof TContext)[]>(todo:
    (...args: { [K in keyof Tokens]: Tokens[K] extends keyof TContext ? TContext[Tokens[K]] : never; }) => void): void;
} 

declare const rootInjector: Injector<{}>;
const fooInjector: Injector<{ foo: string}> = rootInjector;

Output

(none)

Compiler Options

{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

Simpler contravariant examples like this still work as expected.

type Action<T> = (arg: T) => void;
declare let b: Action<{}>;
declare let a: Action<{foo: number}>;
b = a
// => Error! Type 'Action<{ foo: number; }>' is not assignable to type 'Action<{}>'.

[RyanCavanaugh]

Tried replacing method-syntax declarations with arrow function equivalents; no effect.

@nicojs the simpler the repro the sooner we can look at this; any progress would be appreciated

[nicojs]

Thanks for the encouragement @RyanCavanaugh 💖

I've tried to trim it down further, with moderate success:

Playground link

Maybe some more clues. Performance of compiling a project with typed-inject degraded significantly with 3.8 compared to 3.7 (about 3~4 times slower). See the integration test output snippets below.

TypeScript 3.7 (link)

  typed-inject
    ✓ dependency-graph.ts (5308ms)
    ✓ forgot-tokens-class.ts (4536ms)
    ✓ forgot-tokens-function.ts (3395ms)
    ✓ injector-too-variant.ts (3386ms)
    ✓ override-token-should-change-type.ts (3326ms)
    ✓ tokens-of-type-string.ts (3445ms)
    ✓ unknown-token.ts (3562ms)
    ✓ wrong-order-tokens.ts (3209ms)

TypeScript 3.8 (link)

  typed-inject
    ✓ dependency-graph.ts (15980ms)
    ✓ forgot-tokens-class.ts (16109ms)
    ✓ forgot-tokens-function.ts (15598ms)
    1) injector-too-variant.ts
    ✓ override-token-should-change-type.ts (15747ms)
    ✓ tokens-of-type-string.ts (15607ms)
    ✓ unknown-token.ts (15563ms)
    ✓ wrong-order-tokens.ts (15932ms)

[nicojs]

I think I've trimmed it down to the essentials.

playground link

interface Injector<TContext> {
  injectFunction<Tokens extends (keyof TContext)[]>(todo:
    (...args: { [K in keyof Tokens]: Tokens[K] extends keyof TContext ? TContext[Tokens[K]] : never; }) => void): void;
} 

declare const rootInjector: Injector<{}>;
const fooInjector: Injector<{ foo: string}> = rootInjector; // Missing error here in TS 3.8

The error seems to occur when combining a conditional type with spreading parameter lists with tuples. I'll also update the title and the initial message to better reflect this.

[nicojs]

@weswigham or @RyanCavanaugh any updates on this? Would this make it into TypeScript 4.0? 😇

[weswigham]

So I took a peek at this one today, and despite the small repro, there are a few issues going on at once.

First: When comparing instantiations of the same signature (as we do here for the injectFunction method), we erase the method's generics (to any). This is inaccurate, and causes us to always report two instances of an Injector are assignable to one-another (essentially, the outer type parameter, since it's used mostly as a constraint for the method type parameter, just goes away). This is a known flaw in our signature relationship checking, and has a possible fix out at #31029. (You can work around that by including a "phantom data" member on the interface which indicates how the comparison should be performed, eg):

declare const phantomData: unique symbol;
interface Injector<TContext> {
  [phantomData]?: TContext;
  injectFunction<Tokens extends (keyof TContext)[]>(todo:
    (...args: { [K in keyof Tokens]: Tokens[K] extends keyof TContext ? TContext[Tokens[K]] : never; }) => void): void;
} 

declare const rootInjector: Injector<{}>;
const fooInjector: Injector<{ foo: string}> = rootInjector;

Using TContext covariantly in the phantom position will make it covariant, while using it contravariantly (eg, (x: TContext) => void)) will mark it as contravariant.

Second: Once there's a fix or workaround for that in place, this Injector type will fully invert it's behavior - it'll go form accepting all assignments to rejecting all (non-identical) assignments (which, coincidentally, will break the above workaround). The argument comparisons rely on comparing the constraint of the parameter type (string[]) to the still-generic parameter type ({ [K in keyof Tokens]: Tokens[K] extends "foo" ? {foo: string}[Tokens[K]] : never; }). That will currently unconditionally return false, as, even when you peek through the mapped type to the element types (which we do), when we compare string to Tokens[K] extends "foo" ? {foo: string}[Tokens[K]] : never;, we're comparing against a generic conditional, which currently unconditionally returns false. That, in turn, has a possible fix up at #30639.

So this small example seems to hit upon a lot of our relationship checking flaws all at once~

[nicojs]

Thanks for looking into this @weswigham

I'm not following all the details here, but am I correct in assuming that once #30639 and #31029 are merged this issue is fixed? 👐

[weswigham]

Based on my understanding on the workings of the problem, yes.

[nicojs]

@weswigham #31029 is closed! 😨 Is there a follow-up planned? I guess this isn't a "Good first issue" thing?

[weswigham]

I... Dunno. I wasn't even responsible for getting #30639 merged - I'm actually mostly just idly browsing notifications while I'm conscious while in the hospital (heh). @sandersn might know if there's a larger area he's trying to merge fixes in and would better know if #31029 and updating it meets that bar for him.

[sandersn]

I closed #31029 because it had been sitting stale for a year after we decided in the design meeting that it needed some more design work. It's still worthwhile to follow up on the design meeting recommendations and see if they work out.

[nicojs]

Wow! Is it Christmas already? This bug seems to be fixed in TS 4.5. @weswigham can you confirm?

🎄

[RyanCavanaugh]

This does seem fixed 👍