Skip to content

Confusing aspects of recent improvements to Index Types #23994

Closed
@chriseppstein

Description

@chriseppstein

TypeScript Version: HEAD

Search Terms: 23592, index number string keyof

Code

interface OnlyStrings {
  [key: string]: boolean;
}
type OnlyStringsKeys = keyof OnlyStrings; // string | number
let dict: OnlyStrings = {};
dict.hello = true;
dict[42] = false;

interface OnlyNumbers {
  [idx: number]: boolean;
}
type OnlyNumbersKeys = keyof OnlyNumbers; // number
let numbersAllowed: OnlyNumbers = {
  42: false,
  foo: true, // error (as expected)
};
numbersAllowed.foo = true; // error (as expected)
numbersAllowed['foo'] = true; // not an error?? Why is a string literal key allowed but only with brackets?
let s: string = someString();
numbersAllowed[s] = true; // not an error?? why are arbitrary strings allowed as keys?

interface BothStringsAndNumbersLegal {
  [key: string]: boolean;
  [idx: number]: boolean;
}
type BothStringsAndNumbersLegalKeys = keyof OnlyNumbers; // string | number

interface BothStringsAndNumbersNotLegal {
  [keyOrIdx: number | string]: boolean; // Error: An index signature parameter type must be 'string' or 'number'.
}

interface CompleteDiffTypes {
  [key: string]: number;
  [idx: number]: boolean; // error, must be assignable to the of the string type
}

interface NumbersWithDiffType {
  [key: string]: boolean | string;
  [idx: number]: boolean;
}
type StringKeys = NumbersWithDiffType[string]; // boolean | string
type NumericKeys = NumbersWithDiffType[number]; // boolean
let diffTypes: NumbersWithDiffType = {
   foo: true,
   bar: "xxx",
   1: false,
   2: "yyy", // error (as expected)
};
let diffTypes2: NumbersWithDiffType = {}
diffTypes2[1] = false;
diffTypes2[2] = "yyy"; // Error (as expected)

*** Inconsistent Behavior ***

  1. BothStringsAndNumbersNotLegal should be legal, it's just the union type that is returned by keyof. It should be sugar for BothStringsAndNumbersLegal when the values have the same type. I find it strange that I can't set the index type to the exact type that is returned by keyof (without indirection).
  2. Setting a string key into an index type that only allows numbers should always be illegal. The variable numbersAllowed above only gives an error for some of the ways to set a string key onto the interface.

Ok so I don't think the first two are that controversial. But I think the next one will be:

  1. The type OnlyStringsKeys should be string and not string | number OR the types for numbers and strings must be required to be the same.

Yes, I know that is the legacy behavior that is enabled with the special compiler flag. It's a breaking change and that makes it not ideal in the first place. But, beyond that, it's inconsistent.

I presume the rationale is that integers are cast to strings and so the string type has to be a superset of the number type because the values will be merged into the same key space.

But consider this code:

let diffTypes: NumbersWithDiffType = {};
diffTypes[1] = true;
diffTypes["one"] = "astring";
diffTypes["1"] = "ohjoy";
let one = diffTypes[1]; // type inferred as boolean
console.log([one, typeof one]); // => [ 'ohjoy', 'string' ]

It's possible for the merged namespace to cause the type of the string to infect the number type resulting in a value that is incorrectly typed.

I think there's reasonable arguments to be made for one behavior or the other, but I think something has to change.

Any argument that says that the type OnlyStringsKeys has to also include numbers because casting, needs to also say that the value type for number indexes and string indexes has to be the same -- because casting.

If numbers are allowed to have their own type, then the argument must be that the risk of namespace collision for strings/numbers is sufficiently low (or a programmers responsibility to manage). By that argument then strings should also be allowed to have their own type that is not required to be a superset.

The new code for indexed properties as it stands right now is going to cause me to make a new type:

type Keys<Obj extends object> = Extract<keyof Obj, string>;

Which will replace all the uses of keyof Obj for my dictionary types. It's a lot of code changes. I can update this issue with the exact number later. I never use numbers for these types. I don't want to ever pass a number to interfaces unless I said so. But TS allows it:

interface OnlyStrings {
  [key: string]: boolean;
}
let dict: OnlyStrings = {};
dict.hello = true;
dict[42] = false; // not an error :(

If I wanted numbers in my type, I would have said so with the type of the key for the indexed property (by giving two indexes, or a single one with a union type as I suggest above).

If the type checker never allows me to put a number into an object with that interface, then I don't have to worry about namespace collisions. That means the type of keyof T can reasonably be string unless it's declared to allow numbers. This preserves backwards compatibility for keyof. But it breaks backwards compat for interfaces that are declared with a string index, but are setting numbers in them. In my opinion, this is better. Those interfaces should be updated to declare their index type as being string | number, the keyof for those types would then match the intent of that interface. This is the kind of type narrowing backwards incompatibility that I'm used to when I upgrade TS.

I think this change to the keyof types is really great in general, but in its current state on master I'm not a fan of some of these rough edges.

Related Issues: #23592

Metadata

Metadata

Assignees

No one assigned

    Labels

    QuestionAn issue which isn't directly actionable in code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions