Mapped generic type without indexer is assignable to indexer

[russelldavis]

TypeScript Version: 3.5.3, also 3.6.0-dev.20190804

Code

function bug<T>(naked: T, mapped: Partial<T>) {
  let indexer: { [s: string]: unknown };
  // Correctly flagged as an error
  indexer = naked;
  // Bug: typescript lets this assignment succeed
  indexer = mapped;
  // Now I can access an invalid property
  console.log(indexer.notAValidProperty);
}

Expected behavior:
If T is not assignable to an indexer, Partial<T> shouldn't be either.

Playground Link:
Link

Related Issues:
#23080 (old bug where indexers were removed from mapped types)
#28798 (old bug with assignment in the other direction)

[RyanCavanaugh]

This is intentional (checker.ts line 14183):

        if (isGenericMappedType(source)) {
            // A generic mapped type { [P in K]: T } is related to an index signature { [x: string]: U }
            // if T is related to U.
            return (kind === IndexKind.String && isRelatedTo(getTemplateTypeFromMappedType(source), targetInfo.type, reportErrors)) as any as Ternary;
        }

There's a consistency paradox here:

• T should not be assignable to { [s: string] : unknown } because T could be arbitrary
• Partial<unknown> should be assignable to { [s: string] : unknown } because it's a mapped type (comment above)
• Partial<unknown> should not be a less valid assignment source than Partial<T>

[russelldavis]

Partial<unknown> should be assignable to { [s: string] : unknown } because it's a mapped type (comment above)

Hmm, I see the comment that says that's the intention, but it's unclear why it needs to be that way. If the behavior were to change such that Partial<T> is only assignable to an indexer if T is also assignable to an indexer, what would the downside be?

[RyanCavanaugh]

The purpose of an indexer on the assignment target side is to say "Do all properties of this type satisfy this criteria?", where the criteria is "is assignable to this type". When a mapped type produces something that unconditionally satisfies the criteria, there's no logic by which to reject the assignment.

[russelldavis]

Oh man, I could've sworn this behaved differently with a Partial of a concrete type, but I was clearly mistaken. My bad, and thanks for clarifying.

In light of that, I think what we'd actually want here is kind of the opposite of my original proposal: for consistency, it seems like the assignment indexer = naked from my example should actually succeed. Given a criteria of unknown, the answer to "Do all properties of this type satisfy this criteria?" seems equally true for T as it does for Partial<T>.

I know there was a change in 3.5 to make it so { [s: string]: unknown } is no longer a wildcard assignment target, but maybe it still makes sense when the source is a generic type, or something in that vein?

Not a huge deal of course, but the inconsistency here between T and Partial<T> was confusing. In my case, it came up when I noticed that different overloads of Object.entries were being chosen depending on whether the arg type was T or Partial<T>.

[RyanCavanaugh]

I know there was a change in 3.5 to make it so { [s: string]: unknown } is no longer a wildcard assignment target, but maybe it still makes sense when the source is a generic type, or something in that vein?

I'm sympathetic to the consistency argument, but { [s: string]: unknown } and { [s: string]: any } were given different behaviors so that you could choose one based on how you wanted it to behave. If you want an assignable-from-T variant, : any is already there.

[russelldavis]

I'm sympathetic to the consistency argument, but { [s: string]: unknown } and { [s: string]: any } were given different behaviors so that you could choose one based on how you wanted it to behave. If you want an assignable-from-T variant, : any is already there.

Sure, though you don't always have control over which one is used (e.g. Object.entries), and you may want to avoid any for type safety reasons. Ultimately, it's mostly about the consistency. But probably way down in the list of priorities (though these little things do add up).

[typescript-bot]

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.