Allow assignability of non-empty object to generic mapped type

[andrewbranch]

Fixes #31070

The particulars of how I’m doing this feel fairly wrong, but the results seem correct. Not sure why a couple error baselines elaborate way more, but wanted to publish this draft before looking at it to see how off-base this is 😬

Apparently I can't mark this as a draft after the fact

[andrewbranch]

My expectation was that when a mapped type target was instantiated with some T with a constraint like { x: number }, either getConstraintTypeFromMappedType(target) or getIndexType(target) would essentially give me keyof { x: number }. But instead, they both give me a fairly unhelpful index type for a generic object.

So then I thought, “ah, maybe if I use the apparent type of the mapped type,” but in the test case I added, getApparentType(target) === target.

My true intent was just to get a plain object type for that instantiation of that generic mapped type, at which point I think I could just call isRelatedTo(source, plainObjectType).

[weswigham]

If the constraint is keyof U | "a" | "b", getLiteralTypeFromProperties is going to ignore the U-ness of the keyof U part, while the targetConstraint will keep it. We'd like to keep as a keyof (T extends Entity) is different from a keyof (U extends Entity) and needs to be recognized as such. Rather than the filtering using filterType to calculate filteredByApplicability as we're doing now, it's actually probably best to use intersectTypes(targetConstraint, sourceKeys) since the result is the overlapping keys (be they generic or not). Making the indexed access below with that intersection should then work dandily.

In fact, it seems like this is all the changes needed to fix the issue (from master):

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 64bf09d8b3..4476214e71 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -13200,8 +13200,8 @@ namespace ts {
                         if (!isGenericMappedType(source)) {
                             const targetConstraint = getConstraintTypeFromMappedType(target);
                             const sourceKeys = getIndexType(source, /*stringsOnly*/ undefined, /*noIndexSignatures*/ true);
-                            const hasOptionalUnionKeys = modifiers & MappedTypeModifiers.IncludeOptional && targetConstraint.flags & TypeFlags.Union;
-                            const filteredByApplicability = hasOptionalUnionKeys ? filterType(targetConstraint, t => !!isRelatedTo(t, sourceKeys)) : undefined;
+                            const hasOptionalUnionKeys = modifiers & MappedTypeModifiers.IncludeOptional;
+                            const filteredByApplicability = hasOptionalUnionKeys ? intersectTypes(targetConstraint, sourceKeys) : undefined;
                             // A source type T is related to a target type { [P in Q]: X } if Q is related to keyof T and T[Q] is related to X.
                             // A source type T is related to a target type { [P in Q]?: X } if some constituent Q' of Q is related to keyof T and T[Q'] is related to X.
                             if (hasOptionalUnionKeys

[andrewbranch]

Oh, fantastic, that’s so much better 🙌

[weswigham]

If the constraint is keyof U | "a" | "b", getLiteralTypeFromProperties is going to ignore the U-ness of the keyof U part, while the targetConstraint will keep it. We'd like to keep as a keyof (T extends Entity) is different from a keyof (U extends Entity) and needs to be recognized as such. Rather than the filtering using filterType to calculate filteredByApplicability as we're doing now, it's actually probably best to use intersectTypes(targetConstraint, sourceKeys) since the result is the overlapping keys (be they generic or not). Making the indexed access below with that intersection should then work dandily.

In fact, it seems like this is all the changes needed to fix the issue (from master):

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 64bf09d8b3..4476214e71 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -13200,8 +13200,8 @@ namespace ts {
                         if (!isGenericMappedType(source)) {
                             const targetConstraint = getConstraintTypeFromMappedType(target);
                             const sourceKeys = getIndexType(source, /*stringsOnly*/ undefined, /*noIndexSignatures*/ true);
-                            const hasOptionalUnionKeys = modifiers & MappedTypeModifiers.IncludeOptional && targetConstraint.flags & TypeFlags.Union;
-                            const filteredByApplicability = hasOptionalUnionKeys ? filterType(targetConstraint, t => !!isRelatedTo(t, sourceKeys)) : undefined;
+                            const hasOptionalUnionKeys = modifiers & MappedTypeModifiers.IncludeOptional;
+                            const filteredByApplicability = hasOptionalUnionKeys ? intersectTypes(targetConstraint, sourceKeys) : undefined;
                             // A source type T is related to a target type { [P in Q]: X } if Q is related to keyof T and T[Q] is related to X.
                             // A source type T is related to a target type { [P in Q]?: X } if some constituent Q' of Q is related to keyof T and T[Q'] is related to X.
                             if (hasOptionalUnionKeys

[weswigham]

@typescript-bot run dt
@typescript-bot test this

[typescript-bot]

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 27b8c45. You can monitor the build here. It should now contribute to this PR's status checks.

[typescript-bot]

Heya @weswigham, I've started to run the extended test suite on this PR at 27b8c45. You can monitor the build here. It should now contribute to this PR's status checks.

[weswigham]

I think the RWC diff is the same as master, less this branch being out of date with some changes in master, so I think you're good.