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

Add numeric constraint to type parameter of mapped types with name type and array constraints (take 2) #58237

Conversation

Andarist
Copy link
Contributor

fixes #55762

this was already approved by @gabritto and @weswigham and got merged here: #55774 . It was reverted not because it was decided that the change is bad but because it conflicted with another work that was merged before it ( #56727 ) - so it was only reverted because it was merged more recently to main than the other one. Later on that other PR was also reverted (it in fact was the one that was incorrect out of the two) and the issues that it was addressing were fixed by other PRs.

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Apr 18, 2024
@Andarist Andarist force-pushed the numeric-constraint-mapped-type-with-name-type-take-2 branch from 568514f to 9d854aa Compare April 18, 2024 08:22
Comment on lines +48 to +50
const v2_1: number = struct2.a; // error, rest element expands to index signature access
const v2_2: string = struct2.b; // error, rest element expands to index signature access
const v2_3: bigint = struct2.c; // error, rest element expands to index signature access
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is a change from the original #55774 . In the meantime this got merged in #57031 so it only makes sense now to always "deoptimize" this mapping to instantiate~ K with number when any rest element is present in the source tuple.

This isn't very helpful in this situation and maybe it's a reason this PR shouldn't be picked up at all. I don't know - it feels that this suffers from a lack of array/tuple-dedicated syntax for iteration.

The original case with fixed-sized tuples stays improved here though which should be the most popular one anyway.

@gabritto
Copy link
Member

In the midst of fixing some of the problems with mapped types and arrays/tuples that we had, I think we decided that we'd only do the "actually, only iterate through the array/tuple element keys" special behavior for mapped types only if the modifier type is array-like and there's no as clause.
IIUC, that's because that's the scenario where we can be confident the output of the mapped type should be array-like. #57122 improved the first aspect of that (detecting when the modifier type is array-like), and #57093 fixed a problem in the second aspect of that ([K in keyof SomeArray as SomeType]: OtherType<K> no longer considers K constrained by ${number} in the template type).

Note that #57093 means the following has an additional error on the template type side now:

type StructTypeFor<Descriptor extends StructDescriptor> = {
    [K in keyof Descriptor as Descriptor[K][0]]: ValueTypeOf<Descriptor[K][1]>;
};

I don't know, I feel like this PR in particular is sort of like type inference changes: it does what you want in some cases but maybe not what you want in others?

@Andarist
Copy link
Contributor Author

Andarist commented May 1, 2024

@gabritto I agree. This doesn't feel good right now with the recent changes. I mostly re-opened this to get clarity that this is not something that is meant to work. What about the corresponding issue? I think it might be worth closing it now.

@gabritto
Copy link
Member

gabritto commented May 1, 2024

@gabritto I agree. This doesn't feel good right now with the recent changes. I mostly re-opened this to get clarity that this is not something that is meant to work. What about the corresponding issue? I think it might be worth closing it now.

Issue's been updated. Thanks for pointing that out.

@gabritto gabritto closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't remap tuple keys to object keys in mapped types
3 participants