Skip to content

type infer failed with a pathological case in 3.5.0, but in 3.4.0 it works #31385

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
worudso opened this issue May 14, 2019 · 10 comments · Fixed by #31537
Closed

type infer failed with a pathological case in 3.5.0, but in 3.4.0 it works #31385

worudso opened this issue May 14, 2019 · 10 comments · Fixed by #31537
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@worudso
Copy link

worudso commented May 14, 2019

TypeScript Version: 3.5.0-dev.20190509

Search Terms:
generic, keyof
Code

type Foo<T> = {
  [key: string]: { [key_ in keyof T]: key_ }[keyof T]
}
type Bar<T> = {
  [key: string]: { [key_ in keyof T]: [key_] }[keyof T]
}

type Baz<T, Q extends Foo<T>> = {
  [key in keyof Q]: T[Q[key]]
}

type Qux<T, Q extends Bar<T>> = {
  [key in keyof Q]: T[Q[key]["0"]] // error!
}

Expected behavior:
Qux is also a valid generic definition.
Actual behavior:
Type 'Q[key]["0"]' cannot be used to index type 'T'.ts(2536)
Playground Link:
https://www.typescriptlang.org/play/#src=type%20Foo%3CT%3E%20%3D%20%7B%0D%0A%20%20%5Bkey%3A%20string%5D%3A%20%7B%20%5Bkey_%20in%20keyof%20T%5D%3A%20key_%20%7D%5Bkeyof%20T%5D%0D%0A%7D%0D%0Atype%20Bar%3CT%3E%20%3D%20%7B%0D%0A%20%20%5Bkey%3A%20string%5D%3A%20%7B%20%5Bkey_%20in%20keyof%20T%5D%3A%20%5Bkey_%5D%20%7D%5Bkeyof%20T%5D%0D%0A%7D%0D%0A%0D%0Atype%20Baz%3CT%2C%20Q%20extends%20Foo%3CT%3E%3E%20%3D%20%7B%0D%0A%20%20%5Bkey%20in%20keyof%20Q%5D%3A%20T%5BQ%5Bkey%5D%5D%0D%0A%7D%0D%0A%0D%0Atype%20Qux%3CT%2C%20Q%20extends%20Bar%3CT%3E%3E%20%3D%20%7B%0D%0A%20%20%5Bkey%20in%20keyof%20Q%5D%3A%20T%5BQ%5Bkey%5D%5B%220%22%5D%5D%20%2F%2F%20error!%0D%0A%7D

Baz gives the same error here, Qux is ok. Typescript version matters?

Related Issues:

@worudso worudso changed the title type infer failed with a pathological case type infer failed with a pathological case in 3.5.0, but in 3.4.0 it works May 14, 2019
@jack-williams
Copy link
Collaborator

I think this may have changed under #30769, in particular: string indexers are ignored in type variable constraints.

@worudso
Copy link
Author

worudso commented May 14, 2019

@jack-williams
I think I need to take time to read that issue, but asking for the conclusion first,
are you saying that it is sound to say Qux is invalid while Baz is not? So that change was not a side-effect but an improvement?

@jack-williams
Copy link
Collaborator

jack-williams commented May 14, 2019

So the PR I link is the source of the change, but my initial point about string indexers was likely wrong. Corrections (which are probably still wrong in some form, you'll probably need someone on the team to look over this):

I think the change of Baz no longer producing an error is incorrect, however the removal of the error may be attributable to this change:

Constraints of indexed access types are now more thoroughly explored. For example, given type variables T and K extends 'a' | 'b', the types { a: T, b: T }[K] and T are now considered related where previously they weren't.

A reason why there should be an error: In Baz we are saying that all values of Q are assignable to keys of T, but this assumption is not guaranteed by the constraint Foo<T> as evidence by:

declare const sym: unique symbol;
type T = { x: number, b: string };
type Q = { a: "x", [sym]: number };
type A =  Baz<T,Q> // Proof Q is assignable to Foo<T>
type ShouldBeKeyofT<X extends keyof T> = X
type Err = ShouldBeKeyofT<Q[keyof Q]> // err!

The new error in Qux is correct for the same reason that Baz should have been an error. Here is an extended error message that shows the reasoning (note the correct expansion of keyof Q to string | number | symbol)

Type 'Q[keyof Q]["0"]' is not assignable to type 'keyof T'.
  Type 'Q[string]["0"] | Q[number]["0"] | Q[symbol]["0"]' is not assignable to type 'keyof T'.
    Type 'Q[string]["0"]' is not assignable to type 'keyof T'.
      Type '{ [key_ in keyof T]: [key_]; }[keyof T]["0"]' is not assignable to type 'keyof T'.
        Type 'string | number | symbol' is not assignable to type 'keyof T'.
          Type 'string' is not assignable to type 'keyof T'. [2322]

So it seems like the new logic may be eagerly applying { a: T, b: T }[K] relatedTo T when it should be grabbing the base constraint of K and expanding it, but I'm really not sure about that.

@worudso
Copy link
Author

worudso commented May 15, 2019

I can't represent that error in both 3.4.3 and 3.5.0-dev.20190509.
I also found an error occured on type Err = ShouldBeKeyofT<Q[keyof Q]> but ts2344 not ts2322.

Type 'number | "x"' does not satisfy the constraint '"x" | "b"'.
  Type 'number' is not assignable to type '"x" | "b"'.ts(2344)

It can be simply fixed like this, and I think it's an old behavior and has nothing to do with the change.

type Err = ShouldBeKeyofT<Q[string & keyof Q]>

Could you please share me the playground link for your example?

@jack-williams
Copy link
Collaborator

I'm running off master (merged just now). I used this snippet to get the extended error message by assigning to keyof T. The function foo does not error in 3.4 (playground), but does error in master.

function foo<T, Q extends Bar<T>>() {
    const a: Q[keyof Q]["0"] = undefined as any;
    let b: keyof T;
    b = a;
}

@worudso
Copy link
Author

worudso commented May 15, 2019

type X<T> = { [key_ in keyof T]: [key_] }[keyof T];

function foo<T>() {
  const a: X<T>["0"] = undefined as any;
  let b:keyof T;
  b = a; // error
}

type Y<T> = { [key_ in keyof T]: key_ }[keyof T];

function bar<T>() {
  const a: Y<T> = undefined as any;
  let b:keyof T;
  b = a; // ok
}

I saw the same error in 3.5.0-dev.20190515. And there's no error in 3.4.3.
I don't fully understand the unsoundness in foo yet.
But it seems strange to me that only one of them gives error.
Also the expansion({ [key_ in keyof T]: [key_]; }[keyof T]["0"] to string | number | symbol) looks little naive.

@jack-williams
Copy link
Collaborator

Both look like they should be ok to me (so the first seems buggy: it's turning key_ into its constraint), though they are different to your original example. In your original example you are going through generic constraint with a string indexer, here you directly interact with a generic mapped type.

The issue in your first example is that only the string keys of Q are guaranteed to respect the constraint, but in the definitions of Baz and Qux you are assuming that all keys in Q respect the constraint.

@worudso
Copy link
Author

worudso commented May 16, 2019

type Baz<T, Q extends Foo<T>> = {
  [key in string & keyof Q]: T[Q[key]]
}

type Qux<T, Q extends Bar<T>> = {
  [key in string & keyof Q]: T[Q[key]["0"]]
}

Then does it solve the problem? I'm asking if both Baz and Qux should be valid in next version of typescript.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 17, 2019
@worudso
Copy link
Author

worudso commented May 21, 2019

Is this going to be fixed before the release of 3.5.0 or later?

@ahejlsberg
Copy link
Member

Baz and Qux in the original example should both be valid.

Since Q extends Foo<T> and Foo<T> has a string index signature constrained to keyof T, we reason that Q[keyof Q] is assignable to keyof T (i.e. that all properties in Q are constrained to keyof T). This is strictly not correct because Q might have symbol-named properties that aren't subject to the index signature constraint, but we chose to live with that when we extended keyof to include number and symbol named properties.

The only difference between Foo<T> and Bar<T> is the additional nesting in a tuple type and the corresponding fetching of the "0", so that should be valid too. It looks like we're not correctly analyzing the constraint of Q[key]["0"].

@ahejlsberg ahejlsberg added Bug A bug in TypeScript Fixed A PR has been merged for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels May 22, 2019
@ahejlsberg ahejlsberg added this to the TypeScript 3.5.1 milestone May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants