Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

number should be subtype of keyof T[] #13715

Closed
maiermic opened this issue Jan 27, 2017 · 17 comments · Fixed by #23592
Closed

number should be subtype of keyof T[] #13715

maiermic opened this issue Jan 27, 2017 · 17 comments · Fixed by #23592
Labels
Fixed A PR has been merged for this issue

Comments

@maiermic
Copy link

TypeScript Version: 2.1.5

Code

let key: keyof string[]
key = 1 // Type '1' is not assignable to type '"length" | "toString" | "toLocaleString" | "push" | "pop" | "concat" | "join" | "reverse" | "shif...'.
interface Container<T> {
  [index: number]: T
}

let key: keyof Container<string>
key = 1 // Type '1' is not assignable to type 'never'.

Expected behavior:

number should be subtype of keyof string[] and keyof Container<string>

Actual behavior:

number is no subtype of keyof string[] and keyof Container<string>

@mhegazy mhegazy added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jan 27, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Jan 27, 2017

At runtime, any index is really a string. so keyof T is a subtype of string and not string|number. If you are indexing into an array, i would just use number.

@maiermic
Copy link
Author

The following example is mentioned in keyof and Lookup Types:

function getProperty<T, K extends keyof T>(obj: T, key: K) {
    return obj[key];  // Inferred type is T[K]
}

It does not work to access elements of T if T is an array:

getProperty(array, 1) // Type '1' is not assignable to type '"length" | "toString" | "toLocaleString" | "push" | "pop" | "concat" | "join" | "reverse" | "shif...'.

You can solve this by overloading:

function getProperty<E>(obj: E[], key: number): E
function getProperty<T, K extends keyof T>(obj: T, key: K): T[K]
function getProperty(obj: any, key: any): any {
  return obj[key]
}

So I just did what you suggested:

If you are indexing into an array, i would just use number.

However, this does not work, if T is a type variable of an interface:

interface Wrapper<T> {
  getProperty<K extends keyof T>(key: K): Wrapper<T[K]>
}

If T is not an array then number should not be allowed as key (which is the case in the example above). However, if T is an array you can not add an overloaded signature like before, because that invalidates the case of the previous sentence (T is not an array). Besides that, you don't know the element type of T.

interface Wrapper<T> {
  // the overload in the next line should only be applied if T is an array
  getProperty(key: number): any // return type is any since you do not know the element type of T
  getProperty<K extends keyof T>(key: K): Wrapper<T[K]>
}

If number would be subtype of keyof T and T is an array E[] then T[number] would return E. If T is not an array then number is not subtype of keyof T.

@maiermic
Copy link
Author

Moreover, it seems inconsequent to me that keyof { [key: string]: number } is string and keyof { [key: number]: number } is never.

@LennyLixalot
Copy link

LennyLixalot commented Mar 30, 2017

We just hit this limitation ourselves. We're filtering some command objects that have their type identified by a numeric ID and were hoping to have them properly typed in our filter method. This is the workaround we have right now, but as you can see it's ugly. We're passing the ID in as a string instead of a number (yuck) and then converting every command's ID to string to compare it, also yuck.

If numbers were supported in keyof as well as strings, this would just work.

interface Commands {
    [id: number]: IncomingCommand;

    20: IncomingCommand & {
        commandTwentySpecificValue: number
    }
}

function on<TId extends keyof Commands>(id: TId, f: (command: Commands[TId]) => void){
    // Calls f for every command that passes through where potentialCommand.id.toString() === id
}

on("20", command => {
    console.log(command.commandTwentySpecificValue);
});

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 31, 2017

What about adding an overload like this?

function getProperty<T>(obj: { [index: number]: T }, key: number): T

It wouldn't fix all your problems, but it'd work in certain places.

@goloveychuk
Copy link

goloveychuk commented Mar 31, 2017

Faced with this when tried to improve immutablejs methods with typesafety.

interface Record<T> {
  setIn<K1 extends keyof T>(k: [K1], v: T[K1]): void;
  setIn<K1 extends keyof T, K2 extends keyof T[K1]>(k: [K1, K2], v: T[K1][K2]): void;
  setIn<K1 extends keyof T, K2 extends keyof T[K1], K3 extends keyof T[K1][K2]>(k: [K1, K2, K3], v: T[K1][K2][K3]): void;
  setIn<K1 extends keyof T, K2 extends keyof T[K1], K3 extends keyof T[K1][K2], K4 extends keyof T[K1][K2][K3]>(k: [K1, K2, K3, K4], v: T[K1][K2][K3][K4]): void;
  setIn<K1 extends keyof T, K2 extends keyof T[K1], K3 extends keyof T[K1][K2], K4 extends keyof T[K1][K2][K3], K5 extends keyof T[K1][K2][K3][K4]>(k: [K1, K2, K3, K4, K5], v: T[K1][K2][K3][K4][K5]): void;
}

This works nicely until T is nested object. When it'd be Array or Map - keyof can't detect keytype of it.

Maybe solution could be ability to explicitly declare keyof. E.g.

type MyArray = { 
  [keyof]: number
}

Also saw you are planning making variadic generics. Will they help solving this problem?

@mhegazy mhegazy closed this as completed Apr 21, 2017
@LennyLixalot
Copy link

LennyLixalot commented Apr 28, 2017

Why was this closed? Should we take it you never intend to support this feature? Does this count as wont-fix or working-as-intended?

@mhegazy
Copy link
Contributor

mhegazy commented May 1, 2017

The issue was auto-closed since there was no clear action. and has not been touched in more than 14 days.

As noted earlier, number does not exist at runtime, only strings. a solution would be to add a new primitive type numericString that is a subtype of string but is comparable to number, then change all these index signatures to allow for use of numericString and convert number implicitly to numericString. This however seems too complex both from implementation and from usage perspective compared to the value gained by addressing the issue.

@maiermic
Copy link
Author

maiermic commented May 3, 2017

The issue was auto-closed since there was no clear action. and has not been touched in more than 14 days.

@mhegazy Please reopen. In my opinion, this discussion is not done. I guess @goloveychuk and @LennyLixalot do agree. No one reacted to my counter arguments:

They are approved by use cases of @goloveychuk and @LennyLixalot. @goloveychuk's use case refers to the current design limitation regarding generic type variables of an object and @LennyLixalot's use case refers to the mentioned design discrepancy. My suggestion would solve both use cases.

The only argument for the current design decision is

At runtime, any index is really a string. so keyof T is a subtype of string and not string|number. If you are indexing into an array, i would just use number.

I still don't get, why at runtime is a design factor in this case.

The 2 in years[2] is coerced into a string by the JavaScript engine through an implicit toString conversion.
See MDN

If at runtime is crucial, indexing into an array using number wouldn't be allowed. That wouldn't be useful and TypeScript allows number as key of arrays. But wait!

number is no subtype of keyof string[]

Wat

number is not keyof Container<T> even though it is explicitly declared as such:

interface Container<T> {
  [index: number]: T
}

Wat

@mhegazy Please elaborate on this. Why should at runtime still be a design factor in this case, but not in others?

@LennyLixalot
Copy link

Yes, we agree that something should be done about this, it's overly restrictive and has forced us to make a few sub-par design decisions to maintain strong-typing.

@mhegazy
Copy link
Contributor

mhegazy commented May 5, 2017

This how the feature was implemented originally, and we ran into other issues and had to be restricted to a subtype of string. see more details in #12425.

So unless there is a new proposal on how to mitigate these issues but still support indexing with numbers, not sure reopening this issue adds much.

@maiermic
Copy link
Author

maiermic commented May 6, 2017

@mhegazy where are those other issues documented/mentioned? Do you mean #12314? Are there other issues?

Knowing those issues helps to find a proper solution 🙂

@maiermic
Copy link
Author

maiermic commented May 6, 2017

As far as I've seen, all those issues are related to instantiated types, that can not be typed using keyof regarding to @ahejlsberg. That includes:

  • Object.keys
  • Object.entries
  • for..in

They are no valid use case of keyof. It seems to me that keyof is widely misunderstood.

What keyof is not

Given an object obj of actual type O and an interface I with O extends I, implies keyof O extends keyof I. You never absolutely know O in TypeScript. You only know an interface like I. That means you never absolutely know keyof O. You only know keyof I. Object.keys, Object.entries and for..in all use keyof O, which you do not know.

Note: You may argue that O can not contain keys of type number since they are actually strings at runtime. However, the only possible way to access a property of an object is obj[key] and key is coerced to string. So if O accepts all keys of type numericString (like arrays or a lookup type { [key: number]: P }) you can safely pass numbers as key to obj[key] That means, the interface O can contain number as keys even though the actual type O does not contain them. The same applies to number literals. If O has key "0" it accepts key 0.

Summarized, keyof does not absolutely describe all keys of an object at runtime.

What keyof is or what it should be

keyof describes keys of an interface. This interface must not describe everything of an object at runtime. It may only contain a part of it. Further, it should contain keys of type number as stated in the note of the previous section. However, the issue with that is the following:

Given

interface ObjWithKey42<T> {
  42: T
}

and obj of type ObjWithKey42<T>, the expressions obj[42] and obj['42'] are of type T. As a consequence, keyof ObjWithKey42<T> is 42 | '42'. This works for multiple number keys as well:

interface ObjWithKey1To3<T1, T2, T3> {
  1: T1
  2: T2
  3: T3
}

keyof ObjWithKey1To3<T1, T2, T3> is 1 | '1' | 2 | '2' | 3 | '3'. So far, so good. However, we get into trouble with lookup types:

interface Container<T> {
  [index: number]: T
}

We can not list all these numeric string literals (effectively). keyof Container<T> would be number | numericString with numericString as type of all numeric string literals with a corresponding number literal. number | string wouldn't be correct because not every string is a numericString. Also not every string sequens of digit characters is a numbericString since number has a maximum and minimum value.

@mhegazy I guess, that's what you are trying to say.

@mhegazy
Copy link
Contributor

mhegazy commented May 8, 2017

I think so :).

@maiermic
Copy link
Author

maiermic commented Jul 8, 2017

keyof T is always subtype of string. How about something similar: indexof T is always subtype of number:

interface Thing {
    name: string;
    width: number;
    height: number;
    inStock: boolean;
}

type K1 = keyof Thing;  // "name" | "width" | "height" | "inStock"
type I1 = indexof Thing;  // never
type K2 = keyof Thing[];  // "length" | "push" | "pop" | "concat" | ...
type I2 = indexof Thing[];  // number
type K3 = keyof { [x: string]: Thing };  // string
type I3 = indexof { [x: string]: Thing };  // number

type K4 = keyof { [x: number]: Thing };  // never
type I4 = indexof { [x: number]: Thing };  // number
type K5 = keyof { 0: Thing, "1": Thing };  // "0" | "1"
type I5 = indexof { 0: Thing, "1": Thing };  // 0 | 1

@maiermic
Copy link
Author

maiermic commented Jul 8, 2017

indexof combined with keyof would solve the generic issue:

interface Wrapper<T> {
  getProperty<K extends (keyof T | indexof T)>(key: K): Wrapper<T[K]>
}

@airtoxin
Copy link

airtoxin commented Jan 2, 2018

@mhegazy hi, is there any progress?
I also faced same problem.
In JavaScript library, mentioned interface such a getProperty is commonly exists. ex: The best popular library lodash has same interface at #get.
So I really hope to support yielding number type at keyof T[].

ryym added a commit to ryym/wracket that referenced this issue Apr 20, 2018
@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Apr 20, 2018
@mhegazy mhegazy removed the Design Limitation Constraints of the existing architecture prevent this from being fixed label Apr 23, 2018
@mhegazy mhegazy added this to the TypeScript 2.9 milestone Apr 23, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants