Skip to content
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

Improve apparent type of mapped types #57122

Merged
merged 9 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
14 changes: 9 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14542,11 +14542,15 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function getResolvedApparentTypeOfMappedType(type: MappedType) {
const typeVariable = getHomomorphicTypeVariable(type);
if (typeVariable && !type.declaration.nameType) {
const constraint = getConstraintOfTypeParameter(typeVariable);
if (constraint && everyType(constraint, isArrayOrTupleType)) {
return instantiateType(type, prependTypeMapping(typeVariable, constraint, type.mapper));
const target = (type.target ?? type) as MappedType;
const typeVariable = getHomomorphicTypeVariable(target);
if (typeVariable && !target.declaration.nameType) {
const constraint = getConstraintTypeFromMappedType(type);
Copy link
Contributor

Choose a reason for hiding this comment

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

All of this makes sense now... it is tricky that mapped types don't have constraints resolvable by getBaseConstraintOfType and such but we can reach for getConstraintTypeFromMappedType and use that.

if (constraint.flags & TypeFlags.Index) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this check shouldn't be replaced with an assert - it's quite a strong invariant that this branch can only be entered after getHomomorphicTypeVariable returns true and that requires the constraint to be an Index type.

Copy link
Member Author

Choose a reason for hiding this comment

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

An assert here wouldn't be right. There many cases where an instantiation of a homomorphic mapped type doesn't have a keyof XXX constraint anymore. For example, the homomorphic Readonly<T> has the constraint type keyof T, but the instantiation Readonly<{ a: string, b: string }> has the constraint type "a" | "b". What we're looking for here are instantiations where the constraint type is still a keyof XXX. That's what wasn't handled before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation - it makes sense, getHomomorphicTypeVariable is called now with the target and not the type after all.

const baseConstraint = getBaseConstraintOfType((constraint as IndexType).type);
if (baseConstraint && everyType(baseConstraint, isArrayOrTupleType)) {
return instantiateType(target, prependTypeMapping(typeVariable, baseConstraint, type.mapper));
}
}
}
return type;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
//// [tests/cases/compiler/homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts] ////

=== homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts ===
type HandleOptions<O> = {
>HandleOptions : Symbol(HandleOptions, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 0, 0))
>O : Symbol(O, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 0, 19))

[I in keyof O]: {
>I : Symbol(I, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 1, 5))
>O : Symbol(O, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 0, 19))

value: O[I];
>value : Symbol(value, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 1, 21))
>O : Symbol(O, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 0, 19))
>I : Symbol(I, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 1, 5))

};
};

declare function func1<
>func1 : Symbol(func1, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 4, 2))

T extends Record<PropertyKey, readonly any[]>,
>T : Symbol(T, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 6, 23))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))
>PropertyKey : Symbol(PropertyKey, Decl(lib.es5.d.ts, --, --))

>(fields: {
>fields : Symbol(fields, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 8, 2))

[K in keyof T]: {
>K : Symbol(K, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 9, 5))
>T : Symbol(T, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 6, 23))

label: string;
>label : Symbol(label, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 9, 21))

options: [...HandleOptions<T[K]>];
>options : Symbol(options, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 10, 22))
>HandleOptions : Symbol(HandleOptions, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 0, 0))
>T : Symbol(T, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 6, 23))
>K : Symbol(K, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 9, 5))

};
}): T;
>T : Symbol(T, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 6, 23))

const result = func1({
>result : Symbol(result, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 15, 5))
>func1 : Symbol(func1, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 4, 2))

prop: {
>prop : Symbol(prop, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 15, 22))

label: "first",
>label : Symbol(label, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 16, 11))

options: [
>options : Symbol(options, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 17, 23))
{
value: 123,
>value : Symbol(value, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 19, 13))

},
{
value: "foo",
>value : Symbol(value, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 22, 13))

},
],
},
other: {
>other : Symbol(other, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 26, 6))

label: "second",
>label : Symbol(label, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 27, 12))

options: [
>options : Symbol(options, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 28, 24))
{
value: "bar",
>value : Symbol(value, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 30, 13))

},
{
value: true,
>value : Symbol(value, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 33, 13))

},
],
},
});

Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
//// [tests/cases/compiler/homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts] ////

=== homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts ===
type HandleOptions<O> = {
>HandleOptions : HandleOptions<O>

[I in keyof O]: {
value: O[I];
>value : O[I]

};
};

declare function func1<
>func1 : <T extends Record<PropertyKey, readonly any[]>>(fields: { [K in keyof T]: { label: string; options: [...HandleOptions<T[K]>]; }; }) => T

T extends Record<PropertyKey, readonly any[]>,
>(fields: {
>fields : { [K in keyof T]: { label: string; options: [...HandleOptions<T[K]>]; }; }

[K in keyof T]: {
label: string;
>label : string

options: [...HandleOptions<T[K]>];
>options : [...HandleOptions<T[K]>]

};
}): T;

const result = func1({
>result : { prop: [number, string]; other: [string, boolean]; }
>func1({ prop: { label: "first", options: [ { value: 123, }, { value: "foo", }, ], }, other: { label: "second", options: [ { value: "bar", }, { value: true, }, ], },}) : { prop: [number, string]; other: [string, boolean]; }
>func1 : <T extends Record<PropertyKey, readonly any[]>>(fields: { [K in keyof T]: { label: string; options: [...HandleOptions<T[K]>]; }; }) => T
>{ prop: { label: "first", options: [ { value: 123, }, { value: "foo", }, ], }, other: { label: "second", options: [ { value: "bar", }, { value: true, }, ], },} : { prop: { label: string; options: [{ value: number; }, { value: string; }]; }; other: { label: string; options: [{ value: string; }, { value: true; }]; }; }

prop: {
>prop : { label: string; options: [{ value: number; }, { value: string; }]; }
>{ label: "first", options: [ { value: 123, }, { value: "foo", }, ], } : { label: string; options: [{ value: number; }, { value: string; }]; }

label: "first",
>label : string
>"first" : "first"

options: [
>options : [{ value: number; }, { value: string; }]
>[ { value: 123, }, { value: "foo", }, ] : [{ value: number; }, { value: string; }]
{
>{ value: 123, } : { value: number; }

value: 123,
>value : number
>123 : 123

},
{
>{ value: "foo", } : { value: string; }

value: "foo",
>value : string
>"foo" : "foo"

},
],
},
other: {
>other : { label: string; options: [{ value: string; }, { value: true; }]; }
>{ label: "second", options: [ { value: "bar", }, { value: true, }, ], } : { label: string; options: [{ value: string; }, { value: true; }]; }

label: "second",
>label : string
>"second" : "second"

options: [
>options : [{ value: string; }, { value: true; }]
>[ { value: "bar", }, { value: true, }, ] : [{ value: string; }, { value: true; }]
{
>{ value: "bar", } : { value: string; }

value: "bar",
>value : string
>"bar" : "bar"

},
{
>{ value: true, } : { value: true; }

value: true,
>value : true
>true : true

},
],
},
});

126 changes: 126 additions & 0 deletions tests/baselines/reference/ramdaToolsNoInfinite.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
ramdaToolsNoInfinite.ts(115,27): error TS2370: A rest parameter must be of an array type.


==== ramdaToolsNoInfinite.ts (1 errors) ====
// All the following types are explained here:
// https://medium.freecodecamp.org/typescript-curry-ramda-types-f747e99744ab
// https://github.com/pirix-gh/medium/blob/master/types-curry-ramda/src/index.ts
declare namespace Tools {
type Head<T extends any[]> =
T extends [any, ...any[]]
? T[0]
: never;

type Tail<T extends any[]> =
((...t: T) => any) extends ((_: any, ...tail: infer TT) => any)
? TT
: [];

type HasTail<T extends any[]> =
T extends ([] | [any])
? false
: true;

type Last<T extends any[]> = {
0: Last<Tail<T>>;
1: Head<T>;
}[
HasTail<T> extends true
? 0
: 1
];

type Length<T extends any[]> =
T['length'];

type Prepend<E, T extends any[]> =
((head: E, ...args: T) => any) extends ((...args: infer U) => any)
? U
: T;

type Drop<N extends number, T extends any[], I extends any[] = []> = {
0: Drop<N, Tail<T>, Prepend<any, I>>;
1: T;
}[
Length<I> extends N
? 1
: 0
];

type Cast<X, Y> = X extends Y ? X : Y;

type Pos<I extends any[]> =
Length<I>;

type Next<I extends any[]> =
Prepend<any, I>;

type Prev<I extends any[]> =
Tail<I>;

type Iterator<Index extends number = 0, From extends any[] = [], I extends any[] = []> = {
0: Iterator<Index, Next<From>, Next<I>>;
1: From;
}[
Pos<I> extends Index
? 1
: 0
];

type Reverse<T extends any[], R extends any[] = [], I extends any[] = []> = {
0: Reverse<T, Prepend<T[Pos<I>], R>, Next<I>>;
1: R;
}[
Pos<I> extends Length<T>
? 1
: 0
];

type Concat<T1 extends any[], T2 extends any[]> =
Reverse<Reverse<T1> extends infer R ? Cast<R, any[]> : never, T2>;

type Append<E, T extends any[]> =
Concat<T, [E]>;

type ValueOfRecord<R> = R extends Record<any, infer T> ? T : never;
}

declare namespace R {
export type Placeholder = { __placeholder: void };
}

declare namespace Curry {
type GapOf<T1 extends any[], T2 extends any[], TN extends any[], I extends any[]> =
T1[Tools.Pos<I>] extends R.Placeholder
? Tools.Append<T2[Tools.Pos<I>], TN>
: TN;

interface GapsOfWorker<T1 extends any[], T2 extends any[], TN extends any[] = [], I extends any[] = []> {
0: GapsOf<T1, T2, GapOf<T1, T2, TN, I> extends infer G ? Tools.Cast<G, any[]> : never, Tools.Next<I>>;
1: Tools.Concat<TN, Tools.Drop<Tools.Pos<I>, T2> extends infer D ? Tools.Cast<D, any[]> : never>;
}
type GapsOf<T1 extends any[], T2 extends any[], TN extends any[] = [], I extends any[] = []> = GapsOfWorker<T1, T2, TN, I>[
Tools.Pos<I> extends Tools.Length<T1>
? 1
: 0
];

type PartialGaps<T extends any[]> = {
[K in keyof T]?: T[K] | R.Placeholder
};

type CleanedGaps<T extends any[]> = {
[K in keyof T]: NonNullable<T[K]>
};

type Gaps<T extends any[]> = CleanedGaps<PartialGaps<T>>;

type Curry<F extends ((...args: any) => any)> =
<T extends any[]>(...args: Tools.Cast<Tools.Cast<T, Gaps<Parameters<F>>>, any[]>) =>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Copy link
Member

Choose a reason for hiding this comment

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

How is the change in the PR causing this error?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the underlying issue is the same as it was in my PRs. This is now resolved through layers and thus getResolvedApparentTypeOfMappedType sees the constraint of Parameters<F> - that once gets computed as unknown[] (by getDefaultConstraintOfConditionalType when restrictive instantiation bails out from getConstraintOfDistributiveConditionalType) and once as any (by getConstraintOfDistributiveConditionalType). So any can get returned and getResolvedApparentTypeOfMappedType can't recognize this as an array-like. So it ends up not d about it not being assignable to readonly any[] (that is being checked to determine if the type can be used as rest).

IIRC the other case (when unknown[]) is returned makes any[] (added by this Cast) eliminated from the substitution type since, at that point, it's determined that the baseType satisfies the added constraint - so the baseType gets returned.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the issue is mainly in the fact that previously the substitution type wasn't simplified at all here and that & any[] in it (coming from its added constraint) was preserved and made the comparison against readonly any[] OK.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "this is now resolved through layers"?
How do we go from a call to getResolvedApparentTypeOfMappedType to calls to getDefaultConstraintOfConditionalType and getConstraintOfDistributiveConditionalType?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that I'm partially speaking based on what I remember about debugging this last week - so some details might be slightly off.

Gaps is a mapped type - so we enter getResolvedApparentTypeOfMappedType. With this PR we recognize that an instantiation of a homomorphic mapped type is received and we end up checking its constraint and if it's an index type. It turns out it is because it's something like keyof Parameters<...>. So we grab the .type and compute its base constraint. So now we are computing the constraint of a conditional type (through getConstraintFromConditionalType) and, as mentioned above, this once resolves through getDefaultConstraintOfConditionalType (for the restrictive instantiation case) and once through getConstraintOfDistributiveConditionalType (for a regular case :P).

Copy link
Member Author

Choose a reason for hiding this comment

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

With latest commit this error is gone.

!!! error TS2370: A rest parameter must be of an array type.
GapsOf<T, Parameters<F>> extends [any, ...any[]]
? Curry<(...args: GapsOf<T, Parameters<F>> extends infer G ? Tools.Cast<G, any[]> : never) => ReturnType<F>>
: ReturnType<F>;
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// @strict: true
// @noEmit: true

type HandleOptions<O> = {
[I in keyof O]: {
value: O[I];
};
};

declare function func1<
T extends Record<PropertyKey, readonly any[]>,
>(fields: {
[K in keyof T]: {
label: string;
options: [...HandleOptions<T[K]>];
};
}): T;

const result = func1({
prop: {
label: "first",
options: [
{
value: 123,
},
{
value: "foo",
},
],
},
other: {
label: "second",
options: [
{
value: "bar",
},
{
value: true,
},
],
},
});
Loading