Make optional properties assignable to string index signatures

[andrewbranch]

Prerequisite of #41418, discussed in #41505 and #41907.

Attempted release-notes-quality description:

Optional properties are assignable to string index signatures

String index signatures are a way of typing dictionary-like objects, where you want to allow access with arbitrary keys:

const movieWatchCount: { [key: string]: number } = {};

function watchMovie(title: string) {
  movieWatchCount[title] = (movieWatchCount[title] ?? 0) + 1;
}

Of course, for any movie title not yet in the dictionary, movieWatchCount[title] will be undefined. (TypeScript 4.1 added the option --noUncheckedIndexedAccess to include undefined when reading from an index signature like this.) Even though it’s clear that there must be some strings not present in movieWatchCount, previous versions of TypeScript treated optional object properties as unassignable to otherwise compatible index signatures, due to the presence of undefined:

type WesAndersonWatchCount = {
  "Fantastic Mr. Fox"?: number;
  "The Royal Tenenbaums"?: number;
  "Moonrise Kingdom"?: number;
  "The Grand Budapest Hotel"?: number;
};

declare const wesAndersonWatchCount: WesAndersonWatchCount;
const movieWatchCount: { [key: string]: number } = wesAndersonWatchCount;
//    ^^^^^^^^^^^^^^^
//  Type 'WesAndersonWatchCount' is not assignable to type '{ [key: string]: number; }'.
//    Property '"Fantastic Mr. Fox"' is incompatible with index signature.
//      Type 'number | undefined' is not assignable to type 'number'.
//        Type 'undefined' is not assignable to type 'number'. (2322)

TypeScript 4.2 allows this assignment. However, it does not allow the assignment of non-optional properties with undefined in their types, nor does it allow writing undefined to a specific key:

type BatmanWatchCount = {
  "Batman Begins": number | undefined;
  "The Dark Knight": number | undefined;
  "The Dark Knight Rises": number | undefined;
};

declare const batmanWatchCount: BatmanWatchCount;
const movieWatchCount: { [key: string]: number } = batmanWatchCount;
//    ^^^^^^^^^^^^^^^
//  Still an error in 4.2 - only optional properties can be assigned

movieWatchCount["It's the Great Pumpkin, Charlie Brown"] = undefined;
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//  Still an error in 4.2

The new rule also does not apply to number index signatures, since they are assumed to be array-like and dense:

declare let sortOfArrayish: { [key: number]: string };
declare let numberKeys: { 42?: string };
sortOfArrayish = numberKeys;
// ^^^^^^^^^^^
// Type '{ 42?: string | undefined; }' is not assignable to type '{ [key: number]: string; }'.
//   Property '42' is incompatible with index signature.
//     Type 'string | undefined' is not assignable to type 'string'.
//       Type 'undefined' is not assignable to type 'string'.(2322)

[andrewbranch]

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

[typescript-bot]

Heya @andrewbranch, I've started to run the parallelized Definitely Typed test suite on this PR at c88e7f8. You can monitor the build here.

[typescript-bot]

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

[typescript-bot]

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

[andrewbranch]

User tests look good

[andrewbranch]

This is the only mildly interesting thing about this PR. We check if the property type is undefined before filtering undefined out of it in order to keep this error. (Otherwise we would end up checking if never is assignable to string.)

[DanielRosenwasser]

I actually think the ?: string | undefined -> [x: string]: string one is actually weirder.

[DanielRosenwasser]

If the user and RWC tests look good, then awesome!

[andrewbranch]

@typescript-bot test this

[typescript-bot]

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

[andrewbranch]

@typescript-bot test this

[typescript-bot]

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

[Andarist]

Hi! I'm wondering - since this change allows unsound code more often than in the previous versions of TypeScript (playground) I'm wondering if there are maybe some other planned changes (new strict-like flags?) for the optionality of properties? I'm constantly tripped by the fact that an optional property allows undefined to be assigned to it and that a simple in operator check is not able to refine the type of the property by excluding undefined (playground).

[andrewbranch]

@Andarist it looks like you want noUncheckedIndexedAccess.

I don’t think I understand your in example because it looks right to me?

[Andarist]

First of all - thanks for answering!

@Andarist it looks like you want noUncheckedIndexedAccess.

Not quite, this was the demonstration of how this can lead to less sound code and - yes, this flag would help for this particular situation but the problem for me starts a step earlier. The problem is that d3 has been allowed to be assigned to Dict2. I understand that this PR is all about making this assignable - so that's not a problem as I believe this aligns with your design choices. This was kinda a prerequisite to my latter question/observation about TS not recognizing a much observable difference between lack of property and property with undefined value - on which I will continue below. Having the context of TS not seeing much difference between those 2 this change makes sense to me.

I don’t think I understand your in example because it looks right to me?

What I've meant that there is always a difference in my head between an existing property with undefined value and missing property, even though reading that yields undefined in both cases. At runtime, there are not the same though and it would be quite nice if the type system could distinguish those cases. Quite frankly I've come to the conclusion over time that those 2 are pretty much the same for TypeScript in practice:

type A = { a?: string }
type A = { a: string | undefined }

This was most likely not true even before this PR but I've been always struggling to find a practical difference between those two in TS when testing short snippets of code. Might be worth describing this in the docs or something - I've seen people confused by this on multiple occasions.

[andrewbranch]

Note that type A = { a?: string } is syntactic sugar for type A = { a?: string | undefined }. (This is why your in example is supposed to error.) The optionality of the property is still significant, but you’re right that it’s a fundamentally looser check because we don’t have the concept of a missing property type (discussed some in #41505). Another way of looking at it is that there’s an asymmetry here because with non-optional properties, we can enforce that they are present even if they’re allowed to be undefined, but there’s no way to say that some property must be either string or not present on the object at all. (You can do that for fresh object literals via a union type, but can easily get around it by indirect assignment.) But when you combine these limitations / simplified assumptions with --noUncheckedIndexedAccess, I don’t think this PR contributes to unsoundness in the type system. It’s not less sound because your in example correctly errors.

[Andarist]

Thank you for the thorough answer :)

[OliverJAsh]

I just tested this with TypeScript 4.2 and I was hoping it would also fix this error, but it doesn't seem like it does:

type Dict<T> = { [key: string]: T }
interface F extends Dict<string> {
    // Property 'foo' of type 'string | undefined' is not assignable to string index type 'string'.
    foo?: string;
}

Does it make sense to extend this change so that it addresses this error as well?

Also, if I understand correctly, this change is essentially a workaround for a larger issue, which is the fact that TypeScript does not distinguish between missing and undefined. #13195

[andrewbranch]

Well, I think at best, it makes almost as much sense as any of the rest of this stuff, which is to say, I would be very unenthusiastic about widening this hole without a very good reason to do so. I think it probably makes more sense under --noUncheckedIndexedAccess, because there the read type of the index signature would be string | undefined. But it would still kind of be a weird exception to the index signature rule, for dubious gain, I think.