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

Fixed apparent type of homomorphic mapped type with non-homomorphic instantiation #56727

Merged
merged 2 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 20 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2126,6 +2126,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
var amalgamatedDuplicates: Map<string, DuplicateInfoForFiles> | undefined;
var reverseMappedCache = new Map<string, Type | undefined>();
var homomorphicMappedTypeInferenceStack: string[] = [];
var homomorphicMappedTypeApparentTypeStack: Type[] = [];
var ambientModulesCache: Symbol[] | undefined;
/**
* List of every ambient module with a "*" wildcard.
Expand Down Expand Up @@ -14441,11 +14442,26 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function getResolvedApparentTypeOfMappedType(type: MappedType) {
const typeVariable = getHomomorphicTypeVariable(type);
if (typeVariable && !type.declaration.nameType) {
const constraint = getConstraintOfTypeParameter(typeVariable);
if (contains(homomorphicMappedTypeApparentTypeStack, type)) {
return type;
}
const mappedType = type.target as MappedType || type;
const typeVariable = getHomomorphicTypeVariable(mappedType);
if (typeVariable && !mappedType.declaration.nameType) {
let constraint: Type | undefined;
if (!type.target) {
constraint = getConstraintOfTypeParameter(typeVariable);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other branch is now using getApparentType. If that's the correct thing to do then I feel like this probably should call that too. This would fix issues like this one:

declare function func2<A extends readonly any[], B extends A>(
  arr: A,
  fields: [
    // A rest element type must be an array type.(2574)
    ...{
      [K in keyof B]: { v: B[K] };
    },
  ],
): [A, B];

declare const tuple: [number | string, number | boolean, number | bigint];

const result = func2(tuple, [{ v: "foo" }, { v: true }, { v: 6 }]);

Copy link
Member

Choose a reason for hiding this comment

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

You probably could... or you could move the apparent type check forward and replace everyType(constraint, isArrayOrTupleType) with something like everyType(constraint, isApparentArrayOrTupleType), provided it's not critical the instantiation here be with the apparent types (and I don't think the instantiation using an apparent constraint is the important part, since mapped types have lazy apparent type members). It's possible changing the should-this-be-a-mapped-array check we use everywhere to something that looks into the apparent types of the members of the constraint could be prudent.

}
else {
const modifiersConstraint = getConstraintOfType(getModifiersTypeFromMappedType(type));
if (modifiersConstraint) {
homomorphicMappedTypeApparentTypeStack.push(type);
constraint = getApparentType(modifiersConstraint);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not super sure if I should be calling getApparentType here but then... we get to this place from getApparentType so it feels like it's OK to do right now.

homomorphicMappedTypeApparentTypeStack.pop();
}
}
if (constraint && everyType(constraint, isArrayOrTupleType)) {
return instantiateType(type, prependTypeMapping(typeVariable, constraint, type.mapper));
return instantiateType(mappedType, prependTypeMapping(typeVariable, constraint, mappedType.mapper));
}
}
return type;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//// [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, 3))
>O : Symbol(O, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 0, 19))

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

};
};

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, 3))
>T : Symbol(T, Decl(homomorphicMappedTypeWithNonHomomorphicInstantiationSpreadable1.ts, 6, 23))

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

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

};
}): 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, 9))

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

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

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

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

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

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

},
],
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
//// [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

},
],
},
});
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