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

Key in mapped variadic tuple should not be literal after the variadic part #48856

Closed
Josh-Cena opened this issue Apr 27, 2022 · 4 comments Β· Fixed by #57031
Closed

Key in mapped variadic tuple should not be literal after the variadic part #48856

Josh-Cena opened this issue Apr 27, 2022 · 4 comments Β· Fixed by #57031
Labels
Fix Available A PR has been opened for this issue Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Josh-Cena
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

Variadic tuple mapped type keyof

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type Keys<O extends unknown[]> = { [K in keyof O]: K };

type A = Keys<[string, ...string[]]>;
type B = Keys<[string, ...string[], number]>;

πŸ™ Actual behavior

A and B both have the variadic part typed as ..."1"[], which is wrong. B is further wrong because the number part becomes "2", while in practice it would rarely be "2".

πŸ™‚ Expected behavior

A should be ["0", ...number[]]; B should be ["0", ...number[], number]

@andrewbranch andrewbranch added the Bug A bug in TypeScript label Apr 27, 2022
@andrewbranch andrewbranch added this to the TypeScript 4.8.0 milestone Apr 27, 2022
@ahejlsberg
Copy link
Member

I don't think this is a bug. Consider:

type Box<T> = { value: T };
type Boxified<T> = { [P in keyof T]: Box<T[P]> };

type T0 = Boxified<[number, string, ...boolean[]]>;  // [Box<number>, Box<string>, ...Box<boolean>[]]

With the proposed change, P would be number instead of "2" for the rest element, which means the resulting rest element would be ...Box<string | number | boolean>[] instead of just ...Box<boolean[]>. Not only is that less correct, it is a breaking change. Effectively, number is a less precise way of expressing "element at index 2 or above", whereas "2" gets much closer. This speaks to the fact that in homomorphic mappings over array and tuple types, the key types are just exemplars and not an exact representation of some specific property.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Apr 27, 2022
@ahejlsberg ahejlsberg removed this from the TypeScript 4.8.0 milestone Apr 27, 2022
@Josh-Cena
Copy link
Contributor Author

Josh-Cena commented Apr 28, 2022

I also thought about that. The problem is it's really hard to be correct for both the key type itself, and for homomorphic mapped types to continue be homomorphic. However, accessing T[P] and P directly should be two orthogonal actions. In my actual case, I'm doing Entries instead of Keys:

export type Entries<O extends unknown[]> = { [K in keyof O]: [K, O[K]] };

In an ideal world, I'd like Entries<[number, string, ...boolean[]]> to be [["0", number], ["1", string], ...[number, boolean][]]. Does that sound reasonable?

If we have subtracted types, the index should rather by Exclude<number, 0 | 1>, but since we are not there yet, I'd be more happy with number than 2 as the index type.

@typescript-bot
Copy link
Collaborator

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

@ahejlsberg
Copy link
Member

@Josh-Cena FWIW, I now agree with your observations and suggested changes. See #57031. Better late than never! πŸ˜„

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants