Confusing aspects of recent improvements to Index Types

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 ***

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).
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:

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

Anders Hejlsberg (@ahejlsberg)

Anders Hejlsberg (@ahejlsberg) Mohamed Hegazy (@mhegazy) Thoughts?

Some comments on the comments in the code :)

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?

Any object in JS is indexable with string, that is a basic JS feature, and it is modeled in the TS type system. so these operations are allowed the resulting type however is an implicit any.
Both of these are flagged as errors under --noImplicitAny.

I should also add, this is not a new behavior, this has been the behavior since the first TS release.

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

This should be allowed at some point (tracked by #2049). it is important to note that this is just the same as [keyOrIdx: string]: boolean.

Any JS index operation has to go through a string coercion process. so at runtime, anything you use to access a property is first translated to string, then looked up on the object/prototype chain.
TS acknowledges that there are objects, like Array that you always index with number, and indexing into them with string is likely a bug, and not an intended operation. Enter the number indexer, it is an indexer that allows you to index with subsets of strings that are valid numeric values.
so in other words, string indexer subsumes that of a number indexer.

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

keyof is the set of "known" keys for a type. in other words, set of keys the compiler will let you index into the type without a no-implicit-any-error. so if a type has a number index signature, it can be only indexed with number; if it has string index signature, it can be indexed with number or with string.

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

Again, the string indexer is a subset of the number indexer. so dict[42] is treated as dict["42"] for both read and write.

this is the real issue here. we should disallow that, by looking up the numeric value first.

diffTypes["1"] = "ohjoy";

that is technically a breaking change.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) Thanks for the quick reply.

Any object in JS is indexable with string, that is a basic JS feature, and it is modeled in the TS type system... the resulting type however is an implicit any.

I missed the fact that the resulting type was any. I think there is a bug, though, because my project has noImplicitAny enabled and I get no error from let f = numbersAllowed['foo']; VSCode shows me that f has type any, but it's not flagged. On the next line I made a function with a parameter that is implicitly any and I get an error for that.

TS acknowledges that there are objects, like Array that you always index with number, and indexing into them with string is likely a bug, and not an intended operation. ✁...✁ so in other words, string indexer subsumes that of a number indexer.

Does TS, then, also acknowledge there are objects that we never want to index with number and if we did, it's likely a bug? Because it's not my intent that a number should be used as a key for any of the indexed types that I've ever written. I'd like my objects to be type-checked with === semantics, not ==, even if the runtime is more forgiving. This code below doesn't even assign f2 a type of any like the inverse use case did.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy)

we should disallow that, by looking up the numeric value first.

That alone is insufficient, as such an indexing operation would also need to be somehow forbidden in higher order space; otherwise a simple indirection still exhibits the same issue:

interface Obj {
  [x: number]: boolean;
  [x: string]: boolean | string;
}
function set<T extends keyof Obj>(i: Obj, x: T, v: Obj[T]) {
    i[x] = v;
}
const o: Obj = {};
o[1] = true; // OK
const x = set(o, "1", "no!"); // returns `boolean | string`
console.log(typeof o[1]); // now `string`! oh no!

Is this essentially the same problem as the following?

interface DictWithSpecifics {
  [key: string]: boolean | string;
  hello: boolean;
}
let x: DictWithSpecifics = { hello: true };
x["he" + "llo"] = "bad";

Yeah, pretty much. I think the root of the hole is that index signatures are only checked covariantly today (as we only check variance of functions), but because props are read/write by default, technically they should be invariant with respect to the other index signatures and properties they need to be compatable with (excepting a read-only indexer, which already behaves correctly since it would need to be covariant).

Yep, my understanding is that when coming across a computed property access, all the other properties are dropped and assignment is only considered for the indexer. If the type was expressed as an intersection like

type DictWithSpecifics = { [key: string]: boolean | string; } & { hello: boolean; }

then it's essentially eliminating the right branch.

So I think the options might be to either make the computed property type satisfy the intersection of the indexers, or to make the indexers invariant as you suggest.

EDIT: Previous comment was unintelligible as I brought up intersection out of nowhere.

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

Navigation Menu

Repository navigation

Confusing aspects of recent improvements to Index Types #23994

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Confusing aspects of recent improvements to Index Types #23994

Description

Activity

chriseppstein commented on May 9, 2018

mhegazy commented on May 9, 2018

mhegazy commented on May 9, 2018

chriseppstein commented on May 9, 2018

weswigham commented on May 10, 2018

jack-williams commented on May 10, 2018

weswigham commented on May 10, 2018

jack-williams commented on May 10, 2018

typescript-bot commented on Jul 25, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions