Repository navigation
Bug with mapped types and indexing #17238
Copy link
Copy link
Closed
Labels
BugA bug in TypeScriptA bug in TypeScriptFixedA PR has been merged for this issueA PR has been merged for this issue
Milestone
Description
Minimal repro:
type AB = {
a: 'a'
b: 'a'
}
type T1<K extends keyof AB> = { [key in AB[K]]: true }
type T2<K extends keyof AB> = T1<K>[K] // BUG: should be an error for K = 'b'I came across this in production code and made a contrived example to show the issue. Might be related to #15756.
Note that there might be two bugs here in play, one with mapped types and one with lookup types on mapped types. Once I start pushing the types a little, I come across a wild range of unexpected behavior.
Pay attention to the last type 'G', where the compiler is suddenly correct again, which is probably the oddest one of all.
// Let's make a simple type
type StringA = "a"
type A = {
a: StringA
b: StringA
}
// so type A[S] should always resolve to "a" for any valid keys
// note that it doesn't collapse T, even though it could
type T<S extends "a" | "b"> = A[S] // expands A, but doesn't collapse the indexer
let t1: T<"a"> // t1: "a"
let t2: T<"b"> // t1: "a"
// Let's make a silly generic type that only keeps key "a"
// The compiler correctly infers that this new type is just { a: true }
type B<S extends "a" | "b"> = {[key in "a"]: true} // B = { a: true }
// Let's make the type more generic, remember that A[S] is actually just "a"
// The compiler now infers that the type of C is {}, this is plain wrong
type C<S extends "a" | "b"> = {[key in A[S]]: true} // C = {}
// Obviously, we couldn't do a lookup on such a type
// Compiler correctly complains that we can't index an empty object
type D<S extends "a" | "b"> = {}[S] // Type 'S' cannot be used to index type '{}'.
// Now let's add a lookup to the generic version
// Curiously, it doesn't fail this time, but infers 'true', how odd
type E<S extends "a" | "b"> = {[key in A[S]]: true}[S] // = true
// It even happens when we index the above type C, which the compiler claims is {}
type F<S extends "a" | "b"> = C<S>[S] // = true
// Let's also try to use this type, it will become important later
// This does exactly what's expected
let a1: F<"a"> // a: true
let b1: F<"b"> // b: true
// Now let's make it even weirder and add a general indexer
// This correctly doesn't fail as any key is now valid
// The type it infers though, is just plain weird:
// G = ({} & { [key: string]: false; })[S]
type G<S extends "a" | "b"> = (
C<S> &
{[key: string]: false}
)[S]
// And now the weirdest thing of all, let's use this type
// It correctly infers the result! Both of these are correct!
let a2: G<"a"> // a: true
let b2: G<"b"> // b: falseThis has to be the weirdest bug I've run across
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
BugA bug in TypeScriptA bug in TypeScriptFixedA PR has been merged for this issueA PR has been merged for this issue
Activity
gcnew commentedon Jul 17, 2017
It looks like you are experiencing the effects of #12351. The strange behaviour is happening because some of the usecases are expanded, while others are handled on the logical level, without carrying out the heavy lifting.
olegdunkan commentedon Jul 19, 2017
type C<S extends "a" | "b"> = {[key in A[S]]: true}- maybe in this line of code the compiler doesn't evaluateA[S]because of some of sort of lazy evaluation, why does it have to go deeper and analyzeA[S]? But when you typeE<S extends "a" | "b"> = {[key in A[S]]: true}[S]you force compiler to find out exactly what are insideA<T>. It is my assumption.gcnew commentedon Jul 19, 2017
Because of #12351
is converted to just
by applying the above rule, without doing any actual "mapping" or "selection".
PS: You can see the rule is in action, because "extra" keys are silently ignored:
gcnew commentedon Jul 19, 2017
As strange as it may seem
is actually correct. The reason is that
S extends "a" | "b"means thatScan be eithernever"a""b""a" | "b"TypeScript plays it safe here and chooses
never. The "expected" type is returned whenCis instantiated with a specific value forS.Note that the same applies for
S extends keyof A.SimonMeskens commentedon Jul 19, 2017
I disagree that choosing
neveris ever the right choice. Every type has the option of becoming never. If we assume that, then every inferred type everywhere should becomenever.Also, it should never collapse a parametrized type, as you can just wait until it's used and check what the correct value is. A type constraint is just that, a constraint. It should pick the widest type possible for constraints, so in this case, it should pick
"a" | "b"as long as possible.In any case, this behavior is clearly buggy and needs to be fixed.
gcnew commentedon Jul 19, 2017
I disagree. The compiler should (and does) choose the most neutral type. In this case, because at use site the
Sparameter might be any of the above mentioned options, the only safe one is the least restrictive one - i.e.never. If another option was chosen, it would have forced every use site to provide the pertinent properties. If all properties is the intention the right operator iskeyof. When talking about sets of options, the only choice that makes no assumptions is the empty set.SimonMeskens commentedon Jul 19, 2017
So how would you expect someone to write a parameter constrained by string keys?
S extends keyof { a, b }?Would that have different behavior from
S extends "a" | "b"? Sounds like that would still cause buggy behavior, askeyof { a, b }infers to"a" | "b"when collapsed in other places.gcnew commentedon Jul 19, 2017
I'm not sure I understand you. The parameter is still constrained to the said set of keys, it's just that intelli-sense chooses one of the valid options which is the empty set. In subsequent uses of the alias, both value and type level, it works correctly. I'm not convinced that choosing the fullest set in intelli-sense would be better.
SimonMeskens commentedon Jul 19, 2017
Ah, you're just talking about intellisense? I really don't agree. Intellisense should always display the widest possible type. Otherwise you run into cases where intellisense tells you a certain type is
{}and then a line below it, suddenly it widens to a different type.That doesn't change the fact that there's a clear bug going on here though, that needs to be fixed.
gcnew commentedon Jul 19, 2017
Which one do you feel is a bug?
From the comments I'm left with the impression that
and
are the ones you feel are odd, but I think I've explained them in #17238 (comment) and #17238 (comment).
Edit: I've updated wording and gave more context.
SimonMeskens commentedon Jul 19, 2017
I think the example above provides ample amount of bugs, but the main one is this:
How can b1 ever be
true? That's just an outright bug. Type C is exactly the same as type B, because A[S] is always"a". So type C is{ a: true }. b1 tries to index with an index that doesn't even exist on the type it's indexing.gcnew commentedon Jul 19, 2017
You use double indexing.
bresolves to an"a".gcnew commentedon Jul 19, 2017
OK, I see. Yes, it is a bug. The problem is what happens here:
A[S]should still be checked for compatibility before applying the "substitution" rules.SimonMeskens commentedon Jul 19, 2017
I'll add one more example to make it clear:
Adding a bogus indexer to F makes it give different results. I'll edit G above with this version to make it clearer.
SimonMeskens commentedon Jul 19, 2017
I had a small typo in the above example, fixed it, should say
C<S>, notF<S>And you are correct that if E is correctly discarded as an error, the rest of the bugs resulting from that behavior wouldn't matter. I still wanted to document all the behavior as I suspect there is more than one bug at play here.
1 remaining item
gcnew commentedon Jul 19, 2017
BUG 2might have the same cause as #17297.gcnew commentedon Jul 19, 2017
Simon Meskens (@SimonMeskens)
Eis definitely a bug. It's a bit hard to spot though.gcnew commentedon Jul 19, 2017
Simon Meskens (@SimonMeskens) Could you include
in the main post as this is the minimal repro of the problem? I think it will help for clarity.
SimonMeskens commentedon Jul 19, 2017
Will do
I was going to add your previous one with the second bug too. Or should we open a new issue for that? Can you recommend a better title for the bug report too?
gcnew commentedon Jul 19, 2017
The second bug seems to be an incarnation of the same core bug as
bis still an extra key. About the title, I think the current one is fine.SimonMeskens commentedon Jul 19, 2017
Thanks for the help, I didn't know how to get it down to a minimal repro.
jaen commentedon Jul 20, 2017
Out of curiosity, does it indeed relate to #15756 as the OP assumes or is it s disparate issue? Seems related, because it seems like the cause is the simplistic substitution of the type variable (if I understand the thread correctly), but I'm not quite sure.
gcnew commentedon Jul 21, 2017
Simon Meskens (@SimonMeskens) I've put a PR #17336 fixing the issues found in this thread. It turned out that
BUG 2had a different cause and was indeed a second problem.SimonMeskens commentedon Jul 21, 2017
Oh, that's great man! Thanks!