Check for array types when instantiating mapped type constraints with `any` #46218

DanielRosenwasser · 2021-10-05T01:45:37Z

This introduces a change to mapped types so that homomorphic-like mapped types play better with any.

Today, when instantiating a mapped types on keyof T, an instantiation of T with an array-like type behaves differently than with other object types. This allows us to reuse the same mapped types syntax with different kinds of types, but forces us to make a decision when we've hit a type like any. When instantiating with any, we don't consider it to be array-like in any way, and we produce { [x: string]: any }.

type Foo<T extends unknown> = { [K in keyof T]: T[K] };

type X = Foo<any>; // { [x: string]: any }

This is fine in the above example, but is undesirable when constraints would otherwise only allow array-like types.

type Foo<T extends unknown[]> = { [K in keyof T]: T[K] };
//                 ^^^^^^^^^
// Now we're dealing with an array constraint.

type X = Foo<any>; // { [x: string]: any }

The constraints imply that the above mapped type should always produce an array; however, any throws a wrench in the equation. This change introduces a new rule to account for this strange behavior.

Instantiating a mapped type { [K in keyof T]: ... } now produces any[] if the instantiation of T is any, and if T is constrained by only array and tuple types.

This provides better behavior for Promise.all when passing in an argument of type any instead of an array.

Fixes #46169

… array.

… like in `Promise.all`.

DanielRosenwasser · 2021-10-05T01:46:41Z

@typescript-bot pack this
@typescript-bot test this
@typescript-bot user test this

typescript-bot · 2021-10-05T01:46:44Z

Heya @DanielRosenwasser, I've started to run the parallelized community code test suite on this PR at 99d9b56. You can monitor the build here.

typescript-bot · 2021-10-05T01:46:44Z

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 99d9b56. You can monitor the build here.

typescript-bot · 2021-10-05T01:46:44Z

Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 99d9b56. You can monitor the build here.

typescript-bot · 2021-10-05T02:04:30Z

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/112435/artifacts?artifactName=tgz&fileId=77537E9D55CD639A77C2615E5C9FBC90336536082CD0571C13122F50CD35A6B302&fileName=/typescript-4.5.0-insiders.20211005.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.5.0-pr-46218-4".;

typescript-bot · 2021-10-05T02:19:31Z

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

DanielRosenwasser · 2021-10-05T19:18:26Z

tests/baselines/reference/mappedTypeWithAny.errors.txt

+        arr = indirectArrayish;
+        ~~~
+!!! error TS2322: Type 'Objectish<any>' is not assignable to type 'any[]'.


My intent was that IndirectArrayish<U> should be a mapped object type that has a type variable constraint of unknown[]; it seems like that might not be the case, so I actually might need some help here.

I think you'd need a new assignability rule for mapped types matching that pattern to accomplish that.

I think I see what you're saying - that you need to have something recursive that checks if you're arrayish or a generic mapped type that would produce an array result.

function mappedTypeConstraintProducesArrayResult(type: Type) { if (isArrayType(type) || isTupleType(type)) return true; if (type.flags & TypeFlags.Union) return everyType(type, mappedTypeConstraintProducesArrayResult); const typeVariable = type.flags & TypeFlags.MappedType && getHomomorphicTypeVariable(type as MappedType); if (typeVariable && !type.declaration.nameType) { const constraint = getConstraintOfTypeParameter(typeVariable); return !!(constraint && mappedTypeConstraintProducesArrayResult(constraint)); } return false; }

But IndirectArrayish<any> and Objectish<any> aren't generic mapped types, are they?

Uhh, I was thinking of something like relating a generic IndirectArrayish<U> to a U[]. In this case, I don't think we can distinguish between a Objectish<any> and an IndirectArrayish<any> since they're exactly equivalent - the "is this constrained to arrayish types only" check you have is purely syntactic (it only cares if the mapped type was both declared homomorphic and that variable is array-constrained) - it can't pick up information from wrapping declarations or anything.

I guess the intent here was that when we declared IndirectArrayish<U extends unknown[]>, we'd end up with a fresh generic mapped type whose type variable (U) has the array constraint. Is that not what's happening?

weswigham · 2021-10-05T19:21:47Z

src/compiler/checker.ts

@@ -16515,7 +16515,8 @@ namespace ts {
                    return mapTypeWithAlias(getReducedType(mappedTypeVariable), t => {
                        if (t.flags & (TypeFlags.AnyOrUnknown | TypeFlags.InstantiableNonPrimitive | TypeFlags.Object | TypeFlags.Intersection) && t !== wildcardType && !isErrorType(t)) {
                            if (!type.declaration.nameType) {
-                                if (isArrayType(t)) {
+                                let constraint;
+                                if (isArrayType(t) || (t.flags & TypeFlags.Any) && (constraint = getConstraintOfTypeParameter(typeVariable)) && everyType(constraint, or(isArrayType, isTupleType))) {


Suggested change

if (isArrayType(t) || (t.flags & TypeFlags.Any) && (constraint = getConstraintOfTypeParameter(typeVariable)) && everyType(constraint, or(isArrayType, isTupleType))) {

if (isArrayType(t) || (t.flags & TypeFlags.Any) && (constraint = getConstraintOfTypeParameter(typeVariable)) && everyType(constraint, isArrayOrTupleLikeType)) {

? Or is including the x-like types (length properties and 0 properties) going to mess up the behavior in a case we care about? I'm thinking passing any to a T extends {length: number} may also justify an array mapping rather than a string index signature.

I thought about the tuple-like type thing, and that's one concern - I'm already not 100% convinced that that would always be desirable. The bigger concern is the implementation of isArrayLikeType:

return isArrayType(type) || !(type.flags & TypeFlags.Nullable) && isTypeAssignableTo(type, anyReadonlyArrayType);

which I think would consider any an array-like type.

Raw any in a constraint position is eagerly replaced with unknown nowadays (so it behaves as a proper top type constraint and not as an odd anyish thing) so it shouldn't be an issue I don't think.

I think that might be fine, but for now there's some precedent in getResolvedApparentTypeOfMappedType for the current behavior.

treybrisbane · 2021-10-07T02:52:21Z

I don't think it's quite the same, but is this at all related to (or will it have any impact on) #27995?

DanielRosenwasser · 2021-10-07T18:41:19Z

I don't think so - this only affects situations when you have a mapped type over T and instantiate with any in place of T.

ahejlsberg · 2021-10-12T14:23:04Z

The basic change looks good to me and I don't think it's necessary (or even feasible) to check for "arrayishness" in the indirect case. However, the new behavior isn't right for type parameters constrained to tuple types since any[] is not assignable to any tuple types. For those, we really should produce an [any, any, ...] tuple with the appropriate arity, but it gets complicated to deal with optional, rest, and variadic elements in constraints. I think a good middle ground might be to limit the new behavior to cases where every type parameter constraint is an array type and just not do anything special for tuple type constraints.

ahejlsberg · 2021-10-20T15:39:27Z

@DanielRosenwasser Do you agree with the changes I suggest above?

DanielRosenwasser · 2021-10-22T22:43:10Z

Sorry I missed your comment - I agree, and I've updated the PR to only act specially on array types. I've also updated the comment in the test case to explain some of the limitations/concerns we talked about.

DanielRosenwasser · 2021-10-22T23:09:44Z

@ahejlsberg just hit a problem - the whole motivation of the original logic was that the type parameter of Promise.all is constrained to readonly unknown[] | [] - which means that only checking if you have an array type is insufficient.

DanielRosenwasser · 2021-10-26T09:50:05Z

I'm inclined to go back to the original proposal; alternatively, I can change the current implementation to check if the union type contains only array or tuple types, and contains at least one array type.

andrewbranch · 2021-10-26T15:48:20Z

What is “the original proposal”?

if the union type contains only array or tuple types, and contains at least one array type.

This sounds weirdly specific, but not terrible.

DanielRosenwasser · 2021-10-27T09:42:56Z

The original proposal is that we just check if all types are array-like or tuple-like - basically the behavior prior to this commit f01ae16

This reverts commit f01ae16.

DanielRosenwasser · 2021-10-27T12:22:58Z

Latest commits (46283e2) brings that back.

andrewbranch · 2021-10-27T18:40:26Z

@typescript-bot pack this

I know @ahejlsberg had concerns about the correctness of this for tuple types in #46218 (comment).

typescript-bot · 2021-10-27T18:40:30Z

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at b3492a9. You can monitor the build here.

typescript-bot · 2021-10-27T19:01:10Z

Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/113976/artifacts?artifactName=tgz&fileId=801EF1FF477137D59C0165CE8B9A79EE04191A7B7F278368A4FC4753D24A189202&fileName=/typescript-4.5.0-insiders.20211027.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.5.0-pr-46218-18".;

ahejlsberg · 2021-10-27T19:26:49Z

@DanielRosenwasser Am I confused, or did you first implement the change I suggested, and then revert again?

ahejlsberg · 2021-10-27T19:31:08Z

@DanielRosenwasser Ah, I see your reason here. In that case I'm fine with the your original behavior, since our old behavior for tuples wasn't right either.

DanielRosenwasser · 2021-10-27T22:03:10Z

Thanks for the reviews!

sandersn · 2021-10-29T16:17:22Z

This breaks ramda types on DT. I'll file a bug.

… `any` (microsoft#46218) * Added/updated tests. * Accepted baselines. * Update test. * Update instantiateMappedType to work specially when 'any' replaced an array. * Accepted baselines. * Ensure check works when constraint is a union of arrayish types, just like in `Promise.all`. * Accepted baselines. * Update test for indirect instantiation of a mapped type. * Accepted baselines. * Update test comment. * Accepted baselines. * Added tuple test case. * Accepted baselines. * Don't add special behavior for tuples. * Accepted baselines. * Revert "Don't add special behavior for tuples." This reverts commit f01ae16. * Accepted baselines.

DanielRosenwasser added 7 commits October 5, 2021 00:35

Added/updated tests.

ac8be7c

Accepted baselines.

8741cdf

Update test.

151e1a6

Update instantiateMappedType to work specially when 'any' replaced an…

a27f15d

… array.

Accepted baselines.

d8844a7

Ensure check works when constraint is a union of arrayish types, just…

eebb1d4

… like in `Promise.all`.

Accepted baselines.

99d9b56

DanielRosenwasser requested review from weswigham, andrewbranch, ahejlsberg and amcasey October 5, 2021 01:45

typescript-bot assigned DanielRosenwasser and unassigned DanielRosenwasser Oct 5, 2021

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Oct 5, 2021

typescript-bot mentioned this pull request Oct 5, 2021

🤖 User test baselines have changed for mappedTypesOnAny typescript-bot/TypeScript#209

Open

shaonkabir8 approved these changes Oct 5, 2021

View reviewed changes

DanielRosenwasser added 2 commits October 5, 2021 19:13

Update test for indirect instantiation of a mapped type.

9ff4385

Accepted baselines.

6aa3670

DanielRosenwasser commented Oct 5, 2021

View reviewed changes

weswigham reviewed Oct 5, 2021

View reviewed changes

DanielRosenwasser mentioned this pull request Oct 6, 2021

Design Meeting Notes, 10/6/2021 #46247

Closed

sandersn assigned ahejlsberg Oct 19, 2021

DanielRosenwasser added 6 commits October 22, 2021 22:33

Update test comment.

b3ca064

Accepted baselines.

63a6bca

Added tuple test case.

d1939aa

Accepted baselines.

de7f98c

Don't add special behavior for tuples.

f01ae16

Accepted baselines.

f342c82

andrewbranch approved these changes Oct 22, 2021

View reviewed changes

DanielRosenwasser added 2 commits October 27, 2021 12:16

Revert "Don't add special behavior for tuples."

46283e2

This reverts commit f01ae16.

Accepted baselines.

b3492a9

ahejlsberg approved these changes Oct 27, 2021

View reviewed changes

DanielRosenwasser merged commit f494742 into main Oct 27, 2021

DanielRosenwasser deleted the mappedTypesOnAny branch October 27, 2021 22:03

DanielRosenwasser mentioned this pull request Oct 29, 2021

#46218 breaks ramda types on DT #46582

Closed

DanielRosenwasser mentioned this pull request Jan 10, 2024

Mapped type applied to array type produces wrong toString, filter, etc type #57007

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check for array types when instantiating mapped type constraints with `any` #46218

Check for array types when instantiating mapped type constraints with `any` #46218

DanielRosenwasser commented Oct 5, 2021 •

edited

Loading

DanielRosenwasser commented Oct 5, 2021

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021

DanielRosenwasser Oct 5, 2021

weswigham Oct 5, 2021

DanielRosenwasser Oct 5, 2021 •

edited

Loading

weswigham Oct 5, 2021

DanielRosenwasser Oct 5, 2021 •

edited

Loading

weswigham Oct 5, 2021

DanielRosenwasser Oct 5, 2021 •

edited

Loading

weswigham Oct 5, 2021

DanielRosenwasser Oct 5, 2021

treybrisbane commented Oct 7, 2021

DanielRosenwasser commented Oct 7, 2021

ahejlsberg commented Oct 12, 2021

ahejlsberg commented Oct 20, 2021

DanielRosenwasser commented Oct 22, 2021

DanielRosenwasser commented Oct 22, 2021

DanielRosenwasser commented Oct 26, 2021

andrewbranch commented Oct 26, 2021

DanielRosenwasser commented Oct 27, 2021

DanielRosenwasser commented Oct 27, 2021

andrewbranch commented Oct 27, 2021

typescript-bot commented Oct 27, 2021 •

edited

Loading

typescript-bot commented Oct 27, 2021 •

edited

Loading

ahejlsberg commented Oct 27, 2021

ahejlsberg commented Oct 27, 2021

DanielRosenwasser commented Oct 27, 2021

sandersn commented Oct 29, 2021

	if (isArrayType(t) \|\| (t.flags & TypeFlags.Any) && (constraint = getConstraintOfTypeParameter(typeVariable)) && everyType(constraint, or(isArrayType, isTupleType))) {
	if (isArrayType(t) \|\| (t.flags & TypeFlags.Any) && (constraint = getConstraintOfTypeParameter(typeVariable)) && everyType(constraint, isArrayOrTupleLikeType)) {

Check for array types when instantiating mapped type constraints with any #46218

Check for array types when instantiating mapped type constraints with any #46218

Conversation

DanielRosenwasser commented Oct 5, 2021 • edited Loading

DanielRosenwasser commented Oct 5, 2021

typescript-bot commented Oct 5, 2021 • edited Loading

typescript-bot commented Oct 5, 2021 • edited Loading

typescript-bot commented Oct 5, 2021 • edited Loading

typescript-bot commented Oct 5, 2021 • edited Loading

typescript-bot commented Oct 5, 2021

DanielRosenwasser Oct 5, 2021

Choose a reason for hiding this comment

weswigham Oct 5, 2021

Choose a reason for hiding this comment

DanielRosenwasser Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

weswigham Oct 5, 2021

Choose a reason for hiding this comment

DanielRosenwasser Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

weswigham Oct 5, 2021

Choose a reason for hiding this comment

DanielRosenwasser Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

weswigham Oct 5, 2021

Choose a reason for hiding this comment

DanielRosenwasser Oct 5, 2021

Choose a reason for hiding this comment

treybrisbane commented Oct 7, 2021

DanielRosenwasser commented Oct 7, 2021

ahejlsberg commented Oct 12, 2021

ahejlsberg commented Oct 20, 2021

DanielRosenwasser commented Oct 22, 2021

DanielRosenwasser commented Oct 22, 2021

DanielRosenwasser commented Oct 26, 2021

andrewbranch commented Oct 26, 2021

DanielRosenwasser commented Oct 27, 2021

DanielRosenwasser commented Oct 27, 2021

andrewbranch commented Oct 27, 2021

typescript-bot commented Oct 27, 2021 • edited Loading

typescript-bot commented Oct 27, 2021 • edited Loading

ahejlsberg commented Oct 27, 2021

ahejlsberg commented Oct 27, 2021

DanielRosenwasser commented Oct 27, 2021

sandersn commented Oct 29, 2021

Check for array types when instantiating mapped type constraints with `any` #46218

Check for array types when instantiating mapped type constraints with `any` #46218

DanielRosenwasser commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 5, 2021 •

edited

Loading

DanielRosenwasser Oct 5, 2021 •

edited

Loading

DanielRosenwasser Oct 5, 2021 •

edited

Loading

DanielRosenwasser Oct 5, 2021 •

edited

Loading

typescript-bot commented Oct 27, 2021 •

edited

Loading

typescript-bot commented Oct 27, 2021 •

edited

Loading