Fixed crash related to index type deferral on generic mapped types with name types #60528
Conversation
…for their mapped types
…-of-filtering-mapped-types-for-keyof
typescript-bot
added
the
For Uncommitted Bug
| !!! error TS2322: Type 'Mapped6<K>[`_${K}`]' is not assignable to type '`_${string}`'. | ||
| !!! error TS2322: Type 'Mapped6<K>[`_${string}`]' is not assignable to type '`_${string}`'. |
There was a problem hiding this comment.
I believe this change is a fix. The new error matches what was reported by TS 5.0: TS playground
| type Mapped7<K extends string> = { | ||
| [P in K as [P] extends [`_${string}`] ? P : never]: P; | ||
| }; | ||
|
|
||
| function f7<K extends string>(obj: Mapped7<K>, key: keyof Mapped7<K>) { | ||
| let s: `_${string}` = obj[key]; | ||
| } |
There was a problem hiding this comment.
This test case is an extra thing that gets fixed here. This is based on Mapped5+f5 from this file. The only difference between them is that this one is using [P] extends [...] and not P extends ....
This didn't work because the index type deferral was previously dependent on hasDistributiveNameType. I don't quite see a reason why those 2 would type check differently though. Effectively P is always instantiated with a non-union type.
| // skip index type deferral on remapping mapped types | ||
| const objectIndexType = isGenericMappedType(objectType) && getMappedTypeNameTypeKind(objectType) === MappedTypeNameTypeKind.Remapping | ||
| ? getIndexTypeForMappedType(objectType, IndexFlags.None) | ||
| : getIndexType(objectType, IndexFlags.None); |
There was a problem hiding this comment.
This reverts my own change from #55140 . I think now the check wasn't exhaustive anyway and this is now better handled by getSimplifiedIndexType
|
@jakebailey could u run the extended test suite here? :) |
src/compiler/checker.ts
Outdated
| @@ -18350,7 +18356,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| function shouldDeferIndexType(type: Type, indexFlags = IndexFlags.None) { | |||
| return !!(type.flags & TypeFlags.InstantiableNonPrimitive || | |||
| isGenericTupleType(type) || | |||
| isGenericMappedType(type) && (!hasDistributiveNameType(type) || getMappedTypeNameTypeKind(type) === MappedTypeNameTypeKind.Remapping) || | |||
| isGenericMappedType(type) && getNameTypeFromMappedType(type) || | |||
There was a problem hiding this comment.
As mentioned in the comment here:
Effectively P is always instantiated with a non-union type.
Because of that, it's not safe to skip index type deferral for any remapping mapped type as P could be used multiple times by the mapping (and the compiler shouldn't allow for a cross-product of `${P}_${P}`). Checking getMappedTypeNameTypeKind can run into an infinite loop here and it seems the easiest to just avoid checking that and assume that all generic mapped types with name types have to have deferred index types. I compensate for that by simplifying them in relationship checking etc.
| // want to perform the reduction when the name type of a mapped type is distributive with respect to the type variable | ||
| // introduced by the 'in' clause of the mapped type. Note that non-generic types are considered to be distributive because | ||
| // they're the same type regardless of what's being distributed over. | ||
| function hasDistributiveNameType(mappedType: MappedType) { |
There was a problem hiding this comment.
Alternatively, maybe this could be kept but repurposed slightly. It would have to check if used template literal types don't refer to the type variable introduced by 'in' clause more than once. I don't think this is something the compiler would usually do in any other place so I'm hesitant to say that this would be a better solution.
…-of-filtering-mapped-types-for-keyof
…of' into fix/defer-index-types-on-generic-mapped-types-2
typescript-bot
added
For Backlog Bug
There was a problem hiding this comment.
this one got fixed here because now keyof { [K in keyof TActors as K & string]: { src: K; logic: TActors[K]; }; } gets recognized as a deferred index type and thus the logic added in #56742 can kick in
|
@typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
fixes #60476
fixes #56239