-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Leading and middle rest elements in tuple types #41544
Conversation
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 77722bb. You can monitor the build here. Update: The results are in! |
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 77722bb. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 77722bb. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 77722bb. You can monitor the build here. |
@ahejlsberg Here they are:Comparison Report - master..41544
System
Hosts
Scenarios
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
All tests look clean and perf appears unaffected. |
@typescript-bot pack this |
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 77722bb. You can monitor the build here. |
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to pause for a second here to post what I have thus far, since it's mostly design feedback - specifically around the premise of this PR. The first line of the description is:
With this PR we support leading and middle rest elements in tuple types
And that's a really good thing. Except as implemented right here, we only actually support them in generic cases. And then we always end up erasing middle/leading spreads once the generics are gone into a much broader type. If we were erasing them into a much more specific type (so the result allowed fewer assignments), I could maybe get it - but the significantly reduced accuracy of these constructs once we're talking about non-generic types really rubs me the wrong way. I think we need to preserve things like [...string[], ...number[]]
because those types have significant meaning beyond the combined simplifications presented here - the same way the non-generic pattern literals we just added have meaning in the sequences they preserve.
@@ -28,5 +28,5 @@ tests/cases/compiler/spliceTuples.ts(23,1): error TS2322: Type '[number, string, | |||
k6 = [1, ...sbb_]; | |||
~~ | |||
!!! error TS2322: Type '[number, string, boolean, ...boolean[]]' is not assignable to type '[number, string, boolean, boolean, ...boolean[]]'. | |||
!!! error TS2322: Property '3' is optional in type '[number, string, boolean, ...boolean[]]' but required in type '[number, string, boolean, boolean, ...boolean[]]'. | |||
!!! error TS2322: Source provides no match for required element at position 3 in target. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I keep reading this error and coming to it feeling kinda lacking in information. Maybe something like
!!! error TS2322: Source provides no match for required element at position 3 in target. | |
!!! error TS2322: Type '[number, string, boolean, ...boolean[]]' provides no match for required element at position 3 in type '[number, string, boolean, boolean, ...boolean[]]'. | |
I feel like yeah, we work with source/target all the time in the compiler, sure, but I think having the types right there, inline, make it easier to take in. It'd also be neat if we could, like, underline or highlight the tuple element we're referring to somehow, like
!!! error TS2322: Type '[number, string, boolean, ...boolean[]]' provides no match for required element at position 3 in type '[number, string, boolean, boolean, ...boolean[]]'.
~~~~~~~~~~~~ ~~~~~~~
because referring to a tuple element index across two tuples, while concrete, feels like it can be hard to pick out what pair of elements are being compared when the tuples are long. Though that last part (the underlining) we can discuss/consider implementing another time. I do think restoring a slightly more detailed error message is warranted, however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#41860 now keeps track of some ideas we have in mind here.
type Tup3<T extends unknown[], U extends unknown[], V extends unknown[]> = [...T, ...U, ...V]; | ||
|
||
type V20 = Tup3<[number], string[], [number]>; // [number, ...string[], number] | ||
type V21 = Tup3<[number], [string?], [boolean]>; // [number, string | undefined, boolean] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one seems wrong. A [string?]
is equivalent to [] | [string | undefined]
, so I'd expect something equivalent to [number, boolean] | [number, string | undefined, boolean]
to pop out (if not that, verbatim). There's no error here, so quietly swallowing the optionality without reflecting it in the result seems ripe for bad behavior.
|
||
type V20 = Tup3<[number], string[], [number]>; // [number, ...string[], number] | ||
type V21 = Tup3<[number], [string?], [boolean]>; // [number, string | undefined, boolean] | ||
type V22 = Tup3<[number], string[], boolean[]>; // [number, (string | boolean)[]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem equivalent? A sequence of strings followed by a sequence of booleans is certainly different than a mixed sequence of strings and booleans. Specifically, the later admits a type like [number, string, boolean, string, boolean]
that the former does not. Why can we not preserve [number, ...string[], ...boolean[]]
? It certainly seems to me to be distinct from this.
type V20 = Tup3<[number], string[], [number]>; // [number, ...string[], number] | ||
type V21 = Tup3<[number], [string?], [boolean]>; // [number, string | undefined, boolean] | ||
type V22 = Tup3<[number], string[], boolean[]>; // [number, (string | boolean)[]] | ||
type V23 = Tup3<[number], string[], [boolean?]>; // [number, (string | boolean | undefined)[]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we "support" middle rests with this, I'd expect to support them in non-generic scenarios as well - building on my above comments, I'd expect [number, ...string[]] | [number, ...string[], boolean | undefined]
or even [number, ...string[], boolean?]
exactly. The inputs here would seem to insist that we shouldn't admit [number, boolean, string]
, but under the current calculation we do.
type V22 = Tup3<[number], string[], boolean[]>; // [number, (string | boolean)[]] | ||
type V23 = Tup3<[number], string[], [boolean?]>; // [number, (string | boolean | undefined)[]] | ||
type V24 = Tup3<[number], [boolean?], string[]>; // [number, boolean?, ...string[]] | ||
type V25 = Tup3<string[], number[], boolean[]>; // (string | number | boolean)[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, again, the resulting type here is much, much looser than the sequence the inputs imply here. Since we support [...A, ...B, ...C]
in generic cases, we should support preserving that sequencing in cases without generics. The resulting type here should allow assigning ["ok", 0, true]
to it, but not [true, 0, "ok"]
. We really should preserve that sequencing information that we're using on generics in the nongeneric cases. Which, in this case, means I think the resulting output type can only be [...string[], ...number[], ...boolean[]]
. I liken this to our pattern literal types. This is very much like a ${string}${number}${boolean}
- that pattern has meaning in its ordering, and shouldn't be collapsed.
type V23 = Tup3<[number], string[], [boolean?]>; // [number, (string | boolean | undefined)[]] | ||
type V24 = Tup3<[number], [boolean?], string[]>; // [number, boolean?, ...string[]] | ||
type V25 = Tup3<string[], number[], boolean[]>; // (string | number | boolean)[] | ||
type V26 = Tup3<string[], number[], [boolean]>; // [...(string | number)[], boolean] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm probably going to stop pointing out the "the result is much looser than the inputs imply because we're not preserving multiple spreads in nongeneric cases" - there's a lot of those in these tests, and I think I've said all I need to at this point.
type V24 = Tup3<[number], [boolean?], string[]>; // [number, boolean?, ...string[]] | ||
type V25 = Tup3<string[], number[], boolean[]>; // (string | number | boolean)[] | ||
type V26 = Tup3<string[], number[], [boolean]>; // [...(string | number)[], boolean] | ||
type V27 = Tup3<[number?], [string], [boolean?]>; // [number | undefined, string, boolean?] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer [number | undefined, string, boolean?] | [string, boolean?]
here, similarly to one of my above comments. I'm not a fan of dropping the arity-optionality without reflecting it in the resulting type.
@weswigham from your comments, it sounds like you want to loosen the restriction in place:
and add some new behavior for both the generic/non-generic cases. For optional elements, I tend to somewhat agree. As a user, I would expect that we might desugar type Foo = [string, string?, number]; into something like type Foo = [string, number] | [string, string, number]; though not the suggested type Foo = [string, number] | [string, string | undefined, number]; How often do we leverage the distinction between optional and potentially undefined when we have the property itself? For multiple rest elements... well, I don't have a strong-enough intuition on that one, but it would be pretty different in behavior. It would line up better with what we have in template string types. |
I guess so. Since you can replicate those situations in cases where we do not issue an error (eg, via variadic concatenation), I think having accurate behavior for them is important. And thus, once you have accurate behavior for those situations, there's not much of a justification in forbidding them in normal circumstances, I think.
Infrequently. In fact, that we don't is pretty much one of the reasons for #41418 being a bug and not "intended behavior". So the inclusion of
Yeah, I just reason that sequences of types in an array aren't that different from sequences of characters (or types) in a string (or template) - so the later being strictly more powerful with respect to what is preserved through concatenation operations rubs me the wrong way a bit - I feel like we've done some of the thinking on how to match up these sequences, and should probably try to do the same here, rather than quietly losing type fidelity in an unsafe way. |
* Support starting and middle rest elements in tuples * Accept new baselines * Include all rest arguments in error span * Accept new baselines * Fix tests * Add new tests * Fix lint errors
With this PR we support leading and middle rest elements in tuple types. For example, we now support the following:
Previously it was not possible to have anything follow a rest element and thus it was not possible to express tuple types that end in a fixed set of elements. Such tuple types are useful for strongly typing functions with variable parameter lists that end in a fixed set of parameters. For example, the following is now permitted:
Note the use of a rest parameter with a tuple type that starts with a rest element. Also note that indexing a tuple type beyond its starting fixed elements (if any) yields a union type of all possible element types (in this case
string | number
). Thus, type assertions are required in the code above.Tuple type layouts are now governed by these rules:
Thus, the supported layouts of tuple types are:
In either layout, zero or more generic variadic elements may be present at any position in the tuple type. Errors are reported on tuple types that don't match the layout rules, but through generic type instantiation it is still possible to create invalid layouts. For this reason, tuple types are normalized following generic type instantiation:
Some examples of normalization:
Fixes #39595.