Skip to content

When calculating inferred infer type constraints, replace conditionals… #48648

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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 25 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13373,10 +13373,34 @@ namespace ts {
(grandParent.parent as ConditionalTypeNode).extendsType === grandParent && (grandParent.parent as ConditionalTypeNode).checkType.kind === SyntaxKind.MappedType &&
((grandParent.parent as ConditionalTypeNode).checkType as MappedTypeNode).type) {
const checkMappedType = (grandParent.parent as ConditionalTypeNode).checkType as MappedTypeNode;
const nodeType = getTypeFromTypeNode(checkMappedType.type!);
let nodeType = getTypeFromTypeNode(checkMappedType.type!);
// Unwrap any top-level conditionals in the template type to their constraints
while (someType(nodeType, isOrHasGenericConditional)) {
const original = nodeType;
nodeType = mapType(nodeType, t => t.flags & TypeFlags.Conditional && getConstraintFromConditionalType(t as ConditionalType) || t);
if (nodeType === original) {
break;
}
}
// Instantiate away the mapped type parameter into the parameter's constraint
inferences = append(inferences, instantiateType(nodeType,
makeUnaryTypeMapper(getDeclaredTypeOfTypeParameter(getSymbolOfNode(checkMappedType.typeParameter)), checkMappedType.typeParameter.constraint ? getTypeFromTypeNode(checkMappedType.typeParameter.constraint) : keyofConstraintType)
));
// This isn't the highest precision constraint we could make, but the alternatives are very invasive and very ugly
// Specifically, it'd be more correct to do something like manufacture
// {[K in U]: WhateverTemplate<K>}[U]
// or equivalently
// U extends infer K extends U ? WhateverTemplate<K> : never
// but both are _very_ complicated types which encounter issues elsewhere in the checker, even in the common case.
// Specifically
// - the first incorrectly simplifies to `WhateverTemplate<U>` in `getSimplifiedType`, removing the notion of
// distribution over keys from the type and making the template resolve incorrectly if it contains conditionals
// - the second has relationship issues; namely conditional types are usually only related by === equality, but
// are over-instantiated today because `isTypeParamaterPossibleReferenced` doesn't filter outer type parameters
// well enough (or at all, from what I've seen - it only filters out parameters which are unreferenced _everywhere_,
// rather than those which are unused within the given node).
//
// Each of those relationship issues, in turn, then cause inference of the `infer` type parameter in question to fail.
Copy link
Member Author

Choose a reason for hiding this comment

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

This comment block is informative for whomever comes back to this in the future - I looked at two other ways to solve the problem (the conditional in the template type executing differently than the mapped type around it would imply when instantiated in the above fashion), but both ran against other issues in the checker. If one of the other's root issues get fixed, I think they'd be much more elegant constructions to return here than this conditional-constraint-walking logic (ideally the first of the two, since it much more closely matches the structure traversed).

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type KeysWithoutStringIndex<T> =

// Only "foo" | "bar" as expected, [string] index signature removed
type test = KeysWithoutStringIndex<{ [index: string]: string; foo: string; bar: 'baz' }>
>test : never
>test : "foo" | "bar"
Copy link
Member Author

Choose a reason for hiding this comment

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

Fun fact - this test already showed the issue, we just weren't looking for it in this test. (Instead we were looking for a constraint error on a different line.)

>index : string
>foo : string
>bar : "baz"
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/inferImpliedConstraintNotExecuted.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [inferImpliedConstraintNotExecuted.ts]
type KnownKeys<T> = {
[K in keyof T]: string extends K ? never : number extends K ? never : K
} extends { [_ in keyof T]: infer U } ? U : never;


interface HasStringKeys {
[s: string]: any;
}

interface ThingWithKeys extends HasStringKeys {
foo: unknown;
bar: unknown;
}

const demo: KnownKeys<ThingWithKeys> = 'foo';

//// [inferImpliedConstraintNotExecuted.js]
var demo = 'foo';
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/compiler/inferImpliedConstraintNotExecuted.ts ===
type KnownKeys<T> = {
>KnownKeys : Symbol(KnownKeys, Decl(inferImpliedConstraintNotExecuted.ts, 0, 0))
>T : Symbol(T, Decl(inferImpliedConstraintNotExecuted.ts, 0, 15))

[K in keyof T]: string extends K ? never : number extends K ? never : K
>K : Symbol(K, Decl(inferImpliedConstraintNotExecuted.ts, 1, 5))
>T : Symbol(T, Decl(inferImpliedConstraintNotExecuted.ts, 0, 15))
>K : Symbol(K, Decl(inferImpliedConstraintNotExecuted.ts, 1, 5))
>K : Symbol(K, Decl(inferImpliedConstraintNotExecuted.ts, 1, 5))
>K : Symbol(K, Decl(inferImpliedConstraintNotExecuted.ts, 1, 5))

} extends { [_ in keyof T]: infer U } ? U : never;
>_ : Symbol(_, Decl(inferImpliedConstraintNotExecuted.ts, 2, 13))
>T : Symbol(T, Decl(inferImpliedConstraintNotExecuted.ts, 0, 15))
>U : Symbol(U, Decl(inferImpliedConstraintNotExecuted.ts, 2, 33))
>U : Symbol(U, Decl(inferImpliedConstraintNotExecuted.ts, 2, 33))


interface HasStringKeys {
>HasStringKeys : Symbol(HasStringKeys, Decl(inferImpliedConstraintNotExecuted.ts, 2, 50))

[s: string]: any;
>s : Symbol(s, Decl(inferImpliedConstraintNotExecuted.ts, 6, 5))
}

interface ThingWithKeys extends HasStringKeys {
>ThingWithKeys : Symbol(ThingWithKeys, Decl(inferImpliedConstraintNotExecuted.ts, 7, 1))
>HasStringKeys : Symbol(HasStringKeys, Decl(inferImpliedConstraintNotExecuted.ts, 2, 50))

foo: unknown;
>foo : Symbol(ThingWithKeys.foo, Decl(inferImpliedConstraintNotExecuted.ts, 9, 47))

bar: unknown;
>bar : Symbol(ThingWithKeys.bar, Decl(inferImpliedConstraintNotExecuted.ts, 10, 17))
}

const demo: KnownKeys<ThingWithKeys> = 'foo';
>demo : Symbol(demo, Decl(inferImpliedConstraintNotExecuted.ts, 14, 5))
>KnownKeys : Symbol(KnownKeys, Decl(inferImpliedConstraintNotExecuted.ts, 0, 0))
>ThingWithKeys : Symbol(ThingWithKeys, Decl(inferImpliedConstraintNotExecuted.ts, 7, 1))

25 changes: 25 additions & 0 deletions tests/baselines/reference/inferImpliedConstraintNotExecuted.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
=== tests/cases/compiler/inferImpliedConstraintNotExecuted.ts ===
type KnownKeys<T> = {
>KnownKeys : KnownKeys<T>

[K in keyof T]: string extends K ? never : number extends K ? never : K
} extends { [_ in keyof T]: infer U } ? U : never;


interface HasStringKeys {
[s: string]: any;
>s : string
}

interface ThingWithKeys extends HasStringKeys {
foo: unknown;
>foo : unknown

bar: unknown;
>bar : unknown
}

const demo: KnownKeys<ThingWithKeys> = 'foo';
>demo : "foo" | "bar"
>'foo' : "foo"

15 changes: 15 additions & 0 deletions tests/cases/compiler/inferImpliedConstraintNotExecuted.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
type KnownKeys<T> = {
[K in keyof T]: string extends K ? never : number extends K ? never : K
} extends { [_ in keyof T]: infer U } ? U : never;


interface HasStringKeys {
[s: string]: any;
}

interface ThingWithKeys extends HasStringKeys {
foo: unknown;
bar: unknown;
}

const demo: KnownKeys<ThingWithKeys> = 'foo';