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

Improve prop type key when mapping over tuple #48599

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Apr 7, 2022

This makes it easier to map over tuples by allowing T[K & number] on the "right side" to preserve the per-position type of the input tuple.

A TS question on Twitter has been posted by @kof here and I was sort of surprised that this solution didn't quite work as expected. So I got curious as to why it doesn't work and I've discovered that the prop type in the mapper is a string... and thus K & number gets reduced to never and then Tuple[never] produces a union of types at all position (which is somewhat surprising to me as with an object type I just get never back).

It seems that this trivial change~ improves this situation considerably and doesn't break any existing test cases. The improvement can be seen in this diff.

TS playground 4.6

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 7, 2022
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@mikearnaldi
Copy link

Interesting, I also use the string index in #48433 maybe should be adapted too

@Andarist
Copy link
Contributor Author

Andarist commented Apr 8, 2022

@mikearnaldi ye, if this PR here lands then the other one should be adjusted too. If yours lands first then I will add some additional tests here, related to your change.

@sandersn
Copy link
Member

It would really be nice to have an associated bug with inline examples. But this does seem like a small change, so .. @weswigham, what do you think?

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Tuple keys being number literals in the template is... probably a fine change? I'm unsure... Thing is, it's now possible to map from the strings back to the numbers (due to an inference change by @rbuckton ) so maybe we should just be OK with mapped types consistently returning the string form of the keys and allowing users to map those into the numbers if they actually need to?

@Andarist
Copy link
Contributor Author

Andarist commented Apr 22, 2022

It would really be nice to have an associated bug with inline examples.

I'll keep that in mind for the future, to always create an associated issue. It just seemed that a change is small and I've shared the context in the PR description (with the playground link and everything) so I kinda wanted to keep it simple and avoid spamming the repo with separate threads~.

Thing is, it's now possible to map from the strings back to the numbers (due to an inference change by @rbuckton ) so maybe we should just be OK with mapped types consistently returning the string form of the keys and allowing users to map those into the numbers if they actually need to?

While possible, I don't think this is that ergonomic from the user perspective - for what seems to be a standard~ use case. This kind of mapping is not too fancy - it's just mapping one tuple to another.

I would also argue that this matches the common expectations of array/tuple keys being numbers (arr[0]). It's possible to access using both strings and numbers but I don't think that many users think in terms of accessing array types with stringified integers.

IMHO, this also matches better how we can expand a tuple to a union of the values:

type A = [0, '1']
type B = A[number] // 0 | ''

@ahejlsberg
Copy link
Member

While I understand the motivation, I don't think it is right to change the iteration type to be numeric. A homomorphic mapped type is supposed to iterate over keyof T (or, in the case of tuples, some subset thereof) and with this change that wouldn't be the case.

Let me think about the scenario a bit and see if I can come up with something better.

@Andarist
Copy link
Contributor Author

We could maintain the mentioned invariant if tuples would have numeric keys representing their indices and not their stringified equivalents. As a user - this would be much closer to how i think about arrays/tuples.

Separately from that - iterating over tuples could be it's own thing (annotated somehow, or using new syntax). A lot of quirks/unexpected behaviors happen because this piggy backs on regular, object-oriented, mapped types

@ahejlsberg
Copy link
Member

We could maintain the mentioned invariant if tuples would have numeric keys representing their indices and not their stringified equivalents. As a user - this would be much closer to how i think about arrays/tuples.

Agree in principle. Except it wouldn't work because tuples also have a numeric index signature which would "absorb" the numeric literal values when you ask for keyof T. For example, keyof [true, false] currently is number | "0" | "1" | ..., and the number would absorb the numeric literals in what you propose.

@Andarist
Copy link
Contributor Author

Andarist commented Apr 22, 2022

Ah yes - I was wondering how this would play out with the index signature after posting my comment. Didn't think about the absorption problem though.

I don't think that the proposed change is that bad though, even without changing the basic case for keyof Tuple. I see your argument about slight semantic disconnect but it feels really minor, to the point that maintaining it is more surprising to a user than diverging from it. keyof is already a little bit special for homomorphic mapped types so extending that special case further doesn't feel overly bad (especially that we both somewhat agree that it matches the standard expectations better).

But also - this probably would be no issue at all if we could introduce a special keyword like indices/indicesof or similar. I strongly believe that mapping over tuples is currently not intuitive for people, I've seen people struggling with it a lot (we could probably dig up dozens of issues that would be closed by such a feature), and even some core features not always working in an intuitive way (like mapping over concrete tuple not being homomorphic, something that is being fixed here). If we would only have a dedicated way to express this then there would be no need for weird-ish & number when mapping over tuples at all.

By the way - if you figure out how you would like to proceed with this, I would love to give a try implementing your suggestion (if only the proposed solution wouldn't be overly complicated, implementation-wise 😉 ).

The proposed keyword could be used like this:

type UnwrapContainers<T extends Container<unknown>[]> = {
-  [K in keyof T]: T[K & number]["value"];
+  [K in indices T]: T[K]["value"];
};

@Andarist
Copy link
Contributor Author

Just added a comment on a related issue to this. The final solution there could be greatly simplified if we'd have an easier way to iterate over tuples position by position.

@ahejlsberg
Copy link
Member

So, I'm putting together a PR that adds the following:

  • Numeric index signatures apply when the index type is ${number}. For example `Foo[][`${number}`] resolves to Foo instead of being an error. This is consistent with our existing rule that index signatures apply when the index type is a numeric string literal. For example Foo[]['123'] already resolves to Foo.
  • Intersections of type string and a template literal type reduce to just the template literal type. For some reason we never had that rule, but we should such that `Foo[][K & `${number}`], where K is some keyof T, can resolve to Foo.

With these changes, your example works when you write

type UnwrapContainers<T extends Container<unknown>[]> = {
    [K in keyof T]: T[K & `${number}`]["value"];
};

I'm also looking at possibly adding an implied `${number}` constraint to K in cases where it iterates over keyof T for some T constrained to an array type.

@Andarist
Copy link
Contributor Author

I agree with the proposed changes - the first two seem like good semantic improvements when we consider existing functionality.

The proposed implied ${number} constraint would mean that those changes wouldn't be needed for this use case to work, right? That's, of course, not a reason to skip introducing them - I'm just making sure that I understand this correctly. With this proposed change the K & `${number}` wouldn't be needed at all and we could simplify further:

type UnwrapContainers<T extends Container<unknown>[]> = {
-    [K in keyof T]: T[K & `${number}`]["value"];
+    [K in keyof T]: T[K]["value"];
};

I guess this would maintain the invariant about K always being a subset of keyof T since:

const b = ['', 9] as const
type Test = `${number}` & keyof typeof b
//   ^? type Test = "0" | "1"

@treybrisbane
Copy link

Thinking about the underlying problem... Is there any appetite for introducing distinct syntax for array/tuple mapping?

For example, something like

type StringifyElements<T extends ReadonlyArray<any>> = [[I in indexof T]: `${T[I]}`];

(doesn't have to be this syntax specifically!)

I realise the scope of such a feature is significantly beyond that of this PR 😅, but...

Over the years, I've used various hacks/workarounds to get the existing mapped type syntax to correctly transform array/tuple elements, and the results have always been fragile/unreliable. I usually end up using recursive types instead, but these are more complex and have their own problems. Having dedicated syntax here would definitely make my life easier!

The current mapped type syntax is a great tool for general key/value transformation. While arrays/tuples are objects, they're a specialised form of object, and array/tuple mapping is a specialised key/value transformation. As such, having specialised syntax for it would make sense in the same way that having specialised syntax for array value literals does.

Thoughts?

@ahejlsberg
Copy link
Member

PR now up in #48837.

@ahejlsberg
Copy link
Member

Thinking about the underlying problem... Is there any appetite for introducing distinct syntax for array/tuple mapping?

With the improvements in #48837 there shouldn't be a need for distinct syntax for array/tuple mapping. Homomorphic mapped types constrained to array types should cover it.

@Andarist
Copy link
Contributor Author

Closing in favor of the mentioned #48837

@Andarist Andarist closed this Apr 25, 2022
@Andarist Andarist deleted the improve-prop-type-key-when-mapping-over-tuple branch April 25, 2022 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants