Skip to content

Include all properties from the mapped modifier type when calculating… #41976

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 27, 2021
Merged

Include all properties from the mapped modifier type when calculating… #41976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
597a9c6
Include all properties from the mapped modifier type when calculating…
weswigham Dec 15, 2020
335a4e3
Merge branch 'master' into include-modifier-index-props
weswigham Jan 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13868,11 +13868,18 @@ namespace ts {
type.resolvedIndexType || (type.resolvedIndexType = createIndexType(type, /*stringsOnly*/ false));
}

function instantiateTypeAsMappedNameType(nameType: Type, type: MappedType, t: Type) {
return instantiateType(nameType, appendTypeMapping(type.mapper, getTypeParameterFromMappedType(type), t));
}

function getIndexTypeForMappedType(type: MappedType, noIndexSignatures: boolean | undefined) {
const constraint = filterType(getConstraintTypeFromMappedType(type), t => !(noIndexSignatures && t.flags & (TypeFlags.Any | TypeFlags.String)));
const nameType = type.declaration.nameType && getTypeFromTypeNode(type.declaration.nameType);
// If the constraint is exclusively string/number/never type(s), we need to pull the property names from the modified type and run them through the `nameType` mapper as well
// since they won't appear in the constraint, due to subtype reducing with the string/number index types
const properties = nameType && everyType(constraint, t => !!(t.flags & (TypeFlags.String | TypeFlags.Number | TypeFlags.Never))) && getPropertiesOfType(getApparentType(getModifiersTypeFromMappedType(type)));
return nameType ?
mapType(constraint, t => instantiateType(nameType, appendTypeMapping(type.mapper, getTypeParameterFromMappedType(type), t))) :
getUnionType([mapType(constraint, t => instantiateTypeAsMappedNameType(nameType, type, t)), mapType(getUnionType(map(properties || emptyArray, p => getLiteralTypeFromProperty(p, TypeFlags.StringOrNumberLiteralOrUnique))), t => instantiateTypeAsMappedNameType(nameType, type, t))]):
constraint;
}

Expand Down
23 changes: 23 additions & 0 deletions tests/baselines/reference/computedTypesKeyofNoIndexSignatureType.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//// [computedTypesKeyofNoIndexSignatureType.ts]
type Compute<A> = { [K in keyof A]: Compute<A[K]>; } & {};

type EqualsTest<T> = <A>() => A extends T ? 1 : 0;
type Equals<A1, A2> = EqualsTest<A2> extends EqualsTest<A1> ? 1 : 0;

type Filter<K, I> = Equals<K, I> extends 1 ? never : K;

type OmitIndex<T, I extends string | number> = {
[K in keyof T as Filter<K, I>]: T[K];
};

type IndexObject = { [key: string]: unknown; };
type FooBar = { foo: "hello"; bar: "world"; };

type WithIndex = Compute<FooBar & IndexObject>; // { [x: string]: {}; foo: "hello"; bar: "world"; } <-- OK
type WithoutIndex = OmitIndex<WithIndex, string>; // { foo: "hello"; bar: "world"; } <-- OK

type FooBarKey = keyof FooBar; // "foo" | "bar" <-- OK
type WithIndexKey = keyof WithIndex; // string | number <-- Expected: string
type WithoutIndexKey = keyof WithoutIndex; // number <-- Expected: "foo" | "bar"

//// [computedTypesKeyofNoIndexSignatureType.js]
83 changes: 83 additions & 0 deletions tests/baselines/reference/computedTypesKeyofNoIndexSignatureType.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
=== tests/cases/compiler/computedTypesKeyofNoIndexSignatureType.ts ===
type Compute<A> = { [K in keyof A]: Compute<A[K]>; } & {};
>Compute : Symbol(Compute, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 0))
>A : Symbol(A, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 13))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 21))
>A : Symbol(A, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 13))
>Compute : Symbol(Compute, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 0))
>A : Symbol(A, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 13))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 21))

type EqualsTest<T> = <A>() => A extends T ? 1 : 0;
>EqualsTest : Symbol(EqualsTest, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 58))
>T : Symbol(T, Decl(computedTypesKeyofNoIndexSignatureType.ts, 2, 16))
>A : Symbol(A, Decl(computedTypesKeyofNoIndexSignatureType.ts, 2, 22))
>A : Symbol(A, Decl(computedTypesKeyofNoIndexSignatureType.ts, 2, 22))
>T : Symbol(T, Decl(computedTypesKeyofNoIndexSignatureType.ts, 2, 16))

type Equals<A1, A2> = EqualsTest<A2> extends EqualsTest<A1> ? 1 : 0;
>Equals : Symbol(Equals, Decl(computedTypesKeyofNoIndexSignatureType.ts, 2, 50))
>A1 : Symbol(A1, Decl(computedTypesKeyofNoIndexSignatureType.ts, 3, 12))
>A2 : Symbol(A2, Decl(computedTypesKeyofNoIndexSignatureType.ts, 3, 15))
>EqualsTest : Symbol(EqualsTest, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 58))
>A2 : Symbol(A2, Decl(computedTypesKeyofNoIndexSignatureType.ts, 3, 15))
>EqualsTest : Symbol(EqualsTest, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 58))
>A1 : Symbol(A1, Decl(computedTypesKeyofNoIndexSignatureType.ts, 3, 12))

type Filter<K, I> = Equals<K, I> extends 1 ? never : K;
>Filter : Symbol(Filter, Decl(computedTypesKeyofNoIndexSignatureType.ts, 3, 68))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 12))
>I : Symbol(I, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 14))
>Equals : Symbol(Equals, Decl(computedTypesKeyofNoIndexSignatureType.ts, 2, 50))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 12))
>I : Symbol(I, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 14))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 12))

type OmitIndex<T, I extends string | number> = {
>OmitIndex : Symbol(OmitIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 55))
>T : Symbol(T, Decl(computedTypesKeyofNoIndexSignatureType.ts, 7, 15))
>I : Symbol(I, Decl(computedTypesKeyofNoIndexSignatureType.ts, 7, 17))

[K in keyof T as Filter<K, I>]: T[K];
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 8, 3))
>T : Symbol(T, Decl(computedTypesKeyofNoIndexSignatureType.ts, 7, 15))
>Filter : Symbol(Filter, Decl(computedTypesKeyofNoIndexSignatureType.ts, 3, 68))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 8, 3))
>I : Symbol(I, Decl(computedTypesKeyofNoIndexSignatureType.ts, 7, 17))
>T : Symbol(T, Decl(computedTypesKeyofNoIndexSignatureType.ts, 7, 15))
>K : Symbol(K, Decl(computedTypesKeyofNoIndexSignatureType.ts, 8, 3))

};

type IndexObject = { [key: string]: unknown; };
>IndexObject : Symbol(IndexObject, Decl(computedTypesKeyofNoIndexSignatureType.ts, 9, 2))
>key : Symbol(key, Decl(computedTypesKeyofNoIndexSignatureType.ts, 11, 22))

type FooBar = { foo: "hello"; bar: "world"; };
>FooBar : Symbol(FooBar, Decl(computedTypesKeyofNoIndexSignatureType.ts, 11, 47))
>foo : Symbol(foo, Decl(computedTypesKeyofNoIndexSignatureType.ts, 12, 15))
>bar : Symbol(bar, Decl(computedTypesKeyofNoIndexSignatureType.ts, 12, 29))

type WithIndex = Compute<FooBar & IndexObject>; // { [x: string]: {}; foo: "hello"; bar: "world"; } <-- OK
>WithIndex : Symbol(WithIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 12, 46))
>Compute : Symbol(Compute, Decl(computedTypesKeyofNoIndexSignatureType.ts, 0, 0))
>FooBar : Symbol(FooBar, Decl(computedTypesKeyofNoIndexSignatureType.ts, 11, 47))
>IndexObject : Symbol(IndexObject, Decl(computedTypesKeyofNoIndexSignatureType.ts, 9, 2))

type WithoutIndex = OmitIndex<WithIndex, string>; // { foo: "hello"; bar: "world"; } <-- OK
>WithoutIndex : Symbol(WithoutIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 14, 47))
>OmitIndex : Symbol(OmitIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 5, 55))
>WithIndex : Symbol(WithIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 12, 46))

type FooBarKey = keyof FooBar; // "foo" | "bar" <-- OK
>FooBarKey : Symbol(FooBarKey, Decl(computedTypesKeyofNoIndexSignatureType.ts, 15, 49))
>FooBar : Symbol(FooBar, Decl(computedTypesKeyofNoIndexSignatureType.ts, 11, 47))

type WithIndexKey = keyof WithIndex; // string | number <-- Expected: string
>WithIndexKey : Symbol(WithIndexKey, Decl(computedTypesKeyofNoIndexSignatureType.ts, 17, 30))
>WithIndex : Symbol(WithIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 12, 46))

type WithoutIndexKey = keyof WithoutIndex; // number <-- Expected: "foo" | "bar"
>WithoutIndexKey : Symbol(WithoutIndexKey, Decl(computedTypesKeyofNoIndexSignatureType.ts, 18, 36))
>WithoutIndex : Symbol(WithoutIndex, Decl(computedTypesKeyofNoIndexSignatureType.ts, 14, 47))

43 changes: 43 additions & 0 deletions tests/baselines/reference/computedTypesKeyofNoIndexSignatureType.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
=== tests/cases/compiler/computedTypesKeyofNoIndexSignatureType.ts ===
type Compute<A> = { [K in keyof A]: Compute<A[K]>; } & {};
>Compute : { [K in keyof A]: { [K in keyof A[K]]: { [K in keyof A[K][K]]: { [K in keyof A[K][K][K]]: { [K in keyof A[K][K][K][K]]: { [K in keyof A[K][K][K][K][K]]: { [K in keyof A[K][K][K][K][K][K]]: { [K in keyof A[K][K][K][K][K][K][K]]: { [K in keyof A[K][K][K][K][K][K][K][K]]: { [K in keyof A[K][K][K][K][K][K][K][K][K]]: { [K in keyof A[K][K][K][K][K][K][K][K][K][K]]: any; }; }; }; }; }; }; }; }; }; }; }

type EqualsTest<T> = <A>() => A extends T ? 1 : 0;
>EqualsTest : EqualsTest<T>

type Equals<A1, A2> = EqualsTest<A2> extends EqualsTest<A1> ? 1 : 0;
>Equals : Equals<A1, A2>

type Filter<K, I> = Equals<K, I> extends 1 ? never : K;
>Filter : Filter<K, I>

type OmitIndex<T, I extends string | number> = {
>OmitIndex : OmitIndex<T, I>

[K in keyof T as Filter<K, I>]: T[K];
};

type IndexObject = { [key: string]: unknown; };
>IndexObject : IndexObject
>key : string

type FooBar = { foo: "hello"; bar: "world"; };
>FooBar : FooBar
>foo : "hello"
>bar : "world"

type WithIndex = Compute<FooBar & IndexObject>; // { [x: string]: {}; foo: "hello"; bar: "world"; } <-- OK
>WithIndex : { [x: string]: {}; foo: "hello"; bar: "world"; }

type WithoutIndex = OmitIndex<WithIndex, string>; // { foo: "hello"; bar: "world"; } <-- OK
>WithoutIndex : OmitIndex<{ [x: string]: {}; foo: "hello"; bar: "world"; }, string>

type FooBarKey = keyof FooBar; // "foo" | "bar" <-- OK
>FooBarKey : keyof FooBar

type WithIndexKey = keyof WithIndex; // string | number <-- Expected: string
>WithIndexKey : string | number

type WithoutIndexKey = keyof WithoutIndex; // number <-- Expected: "foo" | "bar"
>WithoutIndexKey : number | "foo" | "bar"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? WithoutIndex appears, at least in quick info, not to have an index signature at all (not sure if it’s represented differently under the hood), so it seems like you shouldn’t be allowed to index into it with number?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight simplification of the test case, FWIW:

type OmitIndex<T> = {
    [K in keyof T as (string extends K ? never : K)]: T[K];
};

type WithIndex = { [x: string]: unknown, foo: "hello", bar: "world" };
type WithoutIndex = OmitIndex<WithIndex>; // { foo: "hello"; bar: "world"; }                  <-- OK

type WithIndexKey = keyof WithIndex;
type WithoutIndexKey = keyof WithoutIndex; // number          <-- Expected: "foo" | "bar"

Copy link
Member Author

@weswigham weswigham Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguable - the OmitIndex type is only omitting the string key, however, a string index signature adds both a string and number key to the keyof of the type, since a string index signature implies and allows indexing by number!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, but two follow up questions:

  1. Why doesn’t quick info show the number index signature in WithoutIndex?
  2. How should the type { [K in keyof { [key: string]: any }]: K } be understood? Why isn’t it { [x: string]: string | number } or { [x: string]: string & number } or { [x: string]: string, [x: number]: number }?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 1: The keyof logic includes number in the key type, however mapped type mappings iterate over the set of properties/indexes in the original modifiersType directly if available (and homomorphic), leading to the discrepancy.
As for 2: We have no principled answer and our behaviors are arbitrarily chosen. I think I went over the inconsistency at a design meeting just after key mappings were introduced and we decided to stick with all the inconsistencies so we didn't break anyone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels troubling that

declare let withoutIndex: WithoutIndex;
withoutIndex[3];

is an error even though keyof typeof withoutIndex includes number. Is there any way to observe the numberyness of the keys besides keyof?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

type WithIndex = { [x: string]: unknown, foo: "hello", bar: "world" };

type StringOrNumber = keyof WithIndex;

is already how keyof already works, so, we allow

declare const val: WithIndex;

val[12];

val[null as any as number];

The number-yness of string indexers is pretty visible everywhere it can be shown, IMO. I can't think of somewhere we actually forbid using a number on a string index signature... such a location would probably constitute a bug.

Copy link
Member Author

@weswigham weswigham Jan 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is an error even though keyof typeof withoutIndex includes number.

You can blame this on how we construct mapped types. While the keyof result excludes string, mapped type construction actually doesn't use keyof at all for homomorphic mapped types (heh), and instead iterates over the slots visible on the modifiers type. If the string indexer has been filtered out it doesn't check "oh, well, maybe a number indexer is still appropriate given the filter applied to the modifier", it just drops it.

In any case, number is present in the keys is true both before and after this change, so I'm thinking probably doesn't have too much bearing on this change as-is? That could be a separate enhancement/change to how we create mapped types with as clauses.


20 changes: 20 additions & 0 deletions tests/cases/compiler/computedTypesKeyofNoIndexSignatureType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
type Compute<A> = { [K in keyof A]: Compute<A[K]>; } & {};

type EqualsTest<T> = <A>() => A extends T ? 1 : 0;
type Equals<A1, A2> = EqualsTest<A2> extends EqualsTest<A1> ? 1 : 0;

type Filter<K, I> = Equals<K, I> extends 1 ? never : K;

type OmitIndex<T, I extends string | number> = {
[K in keyof T as Filter<K, I>]: T[K];
};

type IndexObject = { [key: string]: unknown; };
type FooBar = { foo: "hello"; bar: "world"; };

type WithIndex = Compute<FooBar & IndexObject>; // { [x: string]: {}; foo: "hello"; bar: "world"; } <-- OK
type WithoutIndex = OmitIndex<WithIndex, string>; // { foo: "hello"; bar: "world"; } <-- OK

type FooBarKey = keyof FooBar; // "foo" | "bar" <-- OK
type WithIndexKey = keyof WithIndex; // string | number <-- Expected: string
type WithoutIndexKey = keyof WithoutIndex; // number <-- Expected: "foo" | "bar"