-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Fixed an issue with apparent mapped type keys #57838
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
1a27fd9
8d18d2d
8c95b0e
04976bf
cad532a
05cc6dd
21f4f7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22895,14 +22895,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |
| return result; | ||
| } | ||
|
|
||
| function getApparentMappedTypeKeys(nameType: Type, targetType: MappedType) { | ||
| const modifiersType = getApparentType(getModifiersTypeFromMappedType(targetType)); | ||
| function getApparentMappedTypeKeys(nameType: Type, mappedType: MappedType) { | ||
| const modifiersType = getApparentType(getModifiersTypeFromMappedType(mappedType)); | ||
| const mappedKeys: Type[] = []; | ||
| forEachMappedTypePropertyKeyTypeAndIndexSignatureKeyType( | ||
| modifiersType, | ||
| TypeFlags.StringOrNumberLiteralOrUnique, | ||
| /*stringsOnly*/ false, | ||
| t => void mappedKeys.push(instantiateType(nameType, appendTypeMapping(targetType.mapper, getTypeParameterFromMappedType(targetType), t))), | ||
| t => void mappedKeys.push(instantiateType(nameType, appendTypeMapping(mappedType.mapper, getTypeParameterFromMappedType(mappedType), t))), | ||
| ); | ||
| return getUnionType(mappedKeys); | ||
| } | ||
|
|
@@ -23277,7 +23277,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |
| // allow assignments of index types of identical (or similar enough) mapped types. | ||
| // eg, `keyof {[X in keyof A]: Obj[X]}` should be assignable to `keyof {[Y in keyof A]: Tup[Y]}` because both map over the same set of keys (`keyof A`). | ||
| // Without this source-side breakdown, a `keyof {[X in keyof A]: Obj[X]}` style type won't be assignable to anything except itself, which is much too strict. | ||
| const sourceMappedKeys = nameType && isMappedTypeWithKeyofConstraintDeclaration(mappedType) ? getApparentMappedTypeKeys(nameType, mappedType) : (nameType || getConstraintTypeFromMappedType(mappedType)); | ||
| let sourceMappedKeys: Type; | ||
| if (nameType && isMappedTypeWithKeyofConstraintDeclaration(mappedType)) { | ||
| sourceMappedKeys = getApparentMappedTypeKeys(nameType, mappedType); | ||
| if (sourceMappedKeys.flags & TypeFlags.Never) { | ||
|
||
| // modifiers type of mapped type is often `unknown`, `keyof unknown` is `never` and that's assignable to everything | ||
| // letting this through is too permissive so we use the apparent type of an index type here instead | ||
| sourceMappedKeys = stringNumberSymbolType; | ||
| } | ||
| } | ||
| else { | ||
| sourceMappedKeys = nameType || getConstraintTypeFromMappedType(mappedType); | ||
| } | ||
| if (result = isRelatedTo(sourceMappedKeys, target, RecursionFlags.Source, reportErrors)) { | ||
| return result; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| keyRemappingKeyofResult.ts(82,3): error TS2322: Type 'string' is not assignable to type 'Extract<keyof { [P in keyof T as T[P] extends string ? P : never]: any; }, string>'. | ||
| keyRemappingKeyofResult.ts(90,3): error TS2322: Type 'string' is not assignable to type 'keyof { [P in keyof T as T[P] extends string ? P : never]: any; }'. | ||
| Type 'string' is not assignable to type 'T[P] extends string ? P : never'. | ||
|
|
||
|
|
||
| ==== keyRemappingKeyofResult.ts (2 errors) ==== | ||
| const sym = Symbol("") | ||
| type Orig = { [k: string]: any, str: any, [sym]: any } | ||
|
|
||
| type Okay = Exclude<keyof Orig, never> | ||
| // type Okay = string | number | typeof sym | ||
|
|
||
| type Remapped = { [K in keyof Orig as {} extends Record<K, any> ? never : K]: any } | ||
| /* type Remapped = { | ||
| str: any; | ||
| [sym]: any; | ||
| } */ | ||
| // no string index signature, right? | ||
|
|
||
| type Oops = Exclude<keyof Remapped, never> | ||
| declare let x: Oops; | ||
| x = sym; | ||
| x = "str"; | ||
| // type Oops = typeof sym <-- what happened to "str"? | ||
|
|
||
| // equivalently, with an unresolved generic (no `exclude` shenanigans, since conditions won't execute): | ||
| function f<T>() { | ||
| type Orig = { [k: string]: any, str: any, [sym]: any } & T; | ||
|
|
||
| type Okay = keyof Orig; | ||
| let a: Okay; | ||
| a = "str"; | ||
| a = sym; | ||
| a = "whatever"; | ||
| // type Okay = string | number | typeof sym | ||
|
|
||
| type Remapped = { [K in keyof Orig as {} extends Record<K, any> ? never : K]: any } | ||
| /* type Remapped = { | ||
| str: any; | ||
| [sym]: any; | ||
| } */ | ||
| // no string index signature, right? | ||
|
|
||
| type Oops = keyof Remapped; | ||
| let x: Oops; | ||
| x = sym; | ||
| x = "str"; | ||
| } | ||
|
|
||
| // and another generic case with a _distributive_ mapping, to trigger a different branch in `getIndexType` | ||
| function g<T>() { | ||
| type Orig = { [k: string]: any, str: any, [sym]: any } & T; | ||
|
|
||
| type Okay = keyof Orig; | ||
| let a: Okay; | ||
| a = "str"; | ||
| a = sym; | ||
| a = "whatever"; | ||
| // type Okay = string | number | typeof sym | ||
|
|
||
| type NonIndex<T extends PropertyKey> = {} extends Record<T, any> ? never : T; | ||
| type DistributiveNonIndex<T extends PropertyKey> = T extends unknown ? NonIndex<T> : never; | ||
|
|
||
| type Remapped = { [K in keyof Orig as DistributiveNonIndex<K>]: any } | ||
| /* type Remapped = { | ||
| str: any; | ||
| [sym]: any; | ||
| } */ | ||
| // no string index signature, right? | ||
|
|
||
| type Oops = keyof Remapped; | ||
| let x: Oops; | ||
| x = sym; | ||
| x = "str"; | ||
| } | ||
|
|
||
| // https://github.com/microsoft/TypeScript/issues/57827 | ||
|
|
||
| type StringKeys<T> = Extract< | ||
| keyof { | ||
| [P in keyof T as T[P] extends string ? P : never]: any; | ||
| }, | ||
| string | ||
| >; | ||
|
|
||
| function test_57827<T>(z: StringKeys<T>) { | ||
| const f: string = z; | ||
| z = "foo"; // error | ||
| ~ | ||
| !!! error TS2322: Type 'string' is not assignable to type 'Extract<keyof { [P in keyof T as T[P] extends string ? P : never]: any; }, string>'. | ||
| } | ||
|
|
||
| type StringKeys2<T> = keyof { | ||
| [P in keyof T as T[P] extends string ? P : never]: any; | ||
| }; | ||
|
|
||
| function h<T>(z: StringKeys2<T>) { | ||
| z = "foo"; // error | ||
| ~ | ||
| !!! error TS2322: Type 'string' is not assignable to type 'keyof { [P in keyof T as T[P] extends string ? P : never]: any; }'. | ||
| !!! error TS2322: Type 'string' is not assignable to type 'T[P] extends string ? P : never'. | ||
| const f: string = z; // ok | ||
| } | ||
|
|
||
| export {}; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it a bit weird that:
keyof unknownisundefined.getApparentMappedTypeKeys. I'd expect it to be insidegetApparentMappedTypeKeys. Isn't there a case wheregetApparentMappedTypeKeysisneverandsourceMappedKeysshould stay asnever?This doesn't look wrong to me, but I can't yet form a good mental model of what's happening, so I don't know what to think. 😅
I'll revisit in a few days and maybe I'll have a better idea of what this all means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we think about
keyof Tas a set of types that can be safely used to access properties ofTinT[keyof T]thennevermakes sense here.neveris the only type that can be safely used askeyof Tafter we instantiateTasunknown.That's what I started with but mapping
nevertoPropertyKeywould break the other call site wheregetApparentMappedTypeKeysis used. In there, it's used to get the target keys. So this would remove the error below:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all has me thinking that the original code that uses
getApparentMappedTypeKeyson the source side is just... wrong?