Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Leading and middle rest elements in tuple types#41544

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ahejlsberg merged 7 commits intomasterfromfix39595
Jan 5, 2021
Merged

Conversation

@ahejlsberg
Copy link
Member

@ahejlsbergahejlsberg commentedNov 15, 2020•
edited
Loading

With this PR we support leading and middle rest elements in tuple types. For example, we now support the following:

typeT1=[...string[],number];// Zero or more strings followed by a numbertypeT2=[number, ...boolean[],string,string];// Number followed by zero or more booleans followed by two strings

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:

functionf1(...args:[...string[],number]){conststrs=args.slice(0,-1)asstring[];constnum=args[args.length-1]asnumber;// ...}f1(5);f1('abc',5);f1('abc','def',5);f1('abc','def',5,6);// Error

Note the use of a rest parameter with a tuple type thatstarts 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 casestring | number). Thus, type assertions are required in the code above.

Tuple type layouts are now governed by these rules:

  • An optional element cannot precede a required element or follow a rest element.
  • Multiple rest elements are not permitted.

Thus, the supported layouts of tuple types are:

  • Zero or more required elements, followed by zero or more optional elements, followed by zero or one rest element, or
  • Zero or more required elements, followed by a rest element, followed by zero or more required elements.

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 arenormalized following generic type instantiation:

  • Optional elements preceding the last required element are turned into required elements.
  • Elements between the first rest element and the last rest or optional element are turned into a single rest element with a union of the element types.

Some examples of normalization:

typeTup3<Textendsunknown[],Uextendsunknown[],Vextendsunknown[]>=[...T, ...U, ...V];typeTN1=Tup3<[number],string[],[number]>;// [number, ...string[], number]typeTN2=Tup3<[number],[string?],[boolean]>;// [number, string | undefined, boolean]typeTN3=Tup3<[number],string[],[boolean?]>;// [number, (string | boolean | undefined)[]]typeTN4=Tup3<[number],string[],boolean[]>;// [number, (string | boolean)[]]typeTN5=Tup3<string[],number[],boolean[]>;// (string | number | boolean)[]

Fixes#39595.

uhyo, krzkaczor, markusjohnsson, Honga1, ulrichb, ifiokjr, cherryblossom000, sindresorhus, Bnaya, yaroslavvoloshchuk-codaio, and 46 more reacted with thumbs up emojiuhyo, Bnaya, TylorS, and fvilante reacted with laugh emojidead-claudia, cameronaziz-tkww, TylorS, omofolarin, and fvilante reacted with hooray emojiresynth1943, fuafa, filipesabella, uhyo, karol-majewski, dubzzz, JustFly1984, ulrichb, Bnaya, TylorS, and 13 more reacted with heart emojicaptain-yossarian, worstpractice, Bnaya, dead-claudia, AviVahl, btoo, zhao5363, TylorS, fvilante, david-shortman, and yurtsiv reacted with rocket emojiBnaya, zhao5363, TylorS, captain-yossarian, dethereum, fvilante, and brainkim reacted with eyes emoji
@ahejlsberg
Copy link
MemberAuthor

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedNov 15, 2020•
edited
Loading

Heya@ahejlsberg, I've started to run the perf test suite on this PR at77722bb. You can monitor the buildhere.

Update:The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commentedNov 15, 2020•
edited
Loading

Heya@ahejlsberg, I've started to run the parallelized community code test suite on this PR at77722bb. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedNov 15, 2020•
edited
Loading

Heya@ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at77722bb. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedNov 15, 2020•
edited
Loading

Heya@ahejlsberg, I've started to run the extended test suite on this PR at77722bb. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..41544

Metricmaster41544DeltaBestWorst
Angular - node (v10.16.3, x64)
Memory used344,475k (± 0.02%)344,506k (± 0.02%)+31k (+ 0.01%)344,327k344,763k
Parse Time1.99s (± 0.63%)1.99s (± 0.71%)+0.00s (+ 0.05%)1.96s2.03s
Bind Time0.83s (± 0.63%)0.83s (± 1.14%)+0.01s (+ 0.72%)0.81s0.85s
Check Time4.97s (± 0.58%)4.97s (± 0.82%)+0.01s (+ 0.12%)4.89s5.06s
Emit Time5.34s (± 0.54%)5.35s (± 0.72%)+0.01s (+ 0.13%)5.30s5.48s
Total Time13.12s (± 0.37%)13.14s (± 0.63%)+0.02s (+ 0.13%)13.04s13.40s
Monaco - node (v10.16.3, x64)
Memory used354,710k (± 0.02%)354,684k (± 0.01%)-26k (- 0.01%)354,571k354,767k
Parse Time1.60s (± 0.44%)1.60s (± 0.59%)+0.00s (+ 0.12%)1.58s1.62s
Bind Time0.73s (± 0.65%)0.73s (± 0.55%)-0.00s (- 0.55%)0.72s0.74s
Check Time5.15s (± 0.69%)5.15s (± 0.37%)-0.00s (- 0.02%)5.10s5.18s
Emit Time2.81s (± 0.75%)2.81s (± 0.60%)+0.00s (+ 0.07%)2.78s2.85s
Total Time10.29s (± 0.52%)10.29s (± 0.21%)-0.00s (- 0.01%)10.26s10.37s
TFS - node (v10.16.3, x64)
Memory used307,863k (± 0.03%)307,868k (± 0.02%)+5k (+ 0.00%)307,764k308,026k
Parse Time1.24s (± 0.60%)1.24s (± 0.81%)-0.00s (- 0.24%)1.21s1.26s
Bind Time0.68s (± 0.85%)0.68s (± 0.53%)+0.00s (+ 0.59%)0.68s0.69s
Check Time4.59s (± 0.68%)4.60s (± 0.79%)+0.01s (+ 0.17%)4.52s4.67s
Emit Time2.94s (± 1.15%)2.95s (± 0.79%)+0.00s (+ 0.10%)2.89s2.99s
Total Time9.46s (± 0.52%)9.47s (± 0.60%)+0.01s (+ 0.14%)9.33s9.60s
material-ui - node (v10.16.3, x64)
Memory used489,238k (± 0.01%)489,183k (± 0.01%)-55k (- 0.01%)489,034k489,299k
Parse Time2.06s (± 0.43%)2.07s (± 0.64%)+0.00s (+ 0.19%)2.04s2.10s
Bind Time0.66s (± 0.91%)0.65s (± 0.95%)-0.01s (- 1.07%)0.64s0.66s
Check Time13.58s (± 0.91%)13.60s (± 0.58%)+0.02s (+ 0.15%)13.46s13.85s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time16.30s (± 0.77%)16.32s (± 0.52%)+0.02s (+ 0.11%)16.16s16.59s
Angular - node (v12.1.0, x64)
Memory used322,356k (± 0.03%)322,349k (± 0.02%)-7k (- 0.00%)322,170k322,505k
Parse Time1.98s (± 0.95%)1.97s (± 0.53%)-0.01s (- 0.40%)1.95s1.99s
Bind Time0.81s (± 0.37%)0.82s (± 0.63%)+0.01s (+ 0.86%)0.81s0.83s
Check Time4.88s (± 0.39%)4.90s (± 0.39%)+0.02s (+ 0.49%)4.85s4.94s
Emit Time5.50s (± 0.53%)5.54s (± 0.89%)+0.04s (+ 0.76%)5.43s5.64s
Total Time13.17s (± 0.35%)13.23s (± 0.54%)+0.06s (+ 0.48%)13.06s13.37s
Monaco - node (v12.1.0, x64)
Memory used336,801k (± 0.01%)336,814k (± 0.02%)+13k (+ 0.00%)336,638k336,950k
Parse Time1.59s (± 0.33%)1.60s (± 0.47%)+0.01s (+ 0.31%)1.58s1.61s
Bind Time0.71s (± 0.73%)0.71s (± 1.15%)+0.00s (+ 0.56%)0.70s0.74s
Check Time4.91s (± 0.46%)4.92s (± 0.44%)+0.01s (+ 0.29%)4.86s4.96s
Emit Time2.88s (± 0.78%)2.86s (± 0.64%)-0.02s (- 0.52%)2.81s2.90s
Total Time10.09s (± 0.30%)10.10s (± 0.32%)+0.00s (+ 0.04%)10.03s10.17s
TFS - node (v12.1.0, x64)
Memory used292,077k (± 0.03%)292,095k (± 0.03%)+18k (+ 0.01%)291,932k292,263k
Parse Time1.26s (± 0.85%)1.26s (± 0.74%)+0.00s (+ 0.24%)1.24s1.29s
Bind Time0.65s (± 0.46%)0.66s (± 1.22%)+0.01s (+ 0.92%)0.64s0.68s
Check Time4.53s (± 0.62%)4.52s (± 0.65%)-0.01s (- 0.29%)4.45s4.56s
Emit Time2.96s (± 0.84%)2.99s (± 1.03%)+0.03s (+ 1.01%)2.93s3.04s
Total Time9.39s (± 0.58%)9.41s (± 0.62%)+0.02s (+ 0.21%)9.29s9.51s
material-ui - node (v12.1.0, x64)
Memory used467,184k (± 0.05%)467,181k (± 0.05%)-3k (- 0.00%)466,205k467,368k
Parse Time2.08s (± 0.65%)2.08s (± 0.36%)+0.00s (+ 0.10%)2.06s2.09s
Bind Time0.64s (± 0.90%)0.64s (± 0.74%)0.00s ( 0.00%)0.63s0.65s
Check Time12.06s (± 1.07%)12.14s (± 0.87%)+0.08s (+ 0.69%)11.97s12.41s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time14.78s (± 0.96%)14.86s (± 0.74%)+0.09s (+ 0.59%)14.68s15.15s
Angular - node (v8.9.0, x64)
Memory used347,023k (± 0.02%)347,043k (± 0.02%)+20k (+ 0.01%)346,898k347,140k
Parse Time2.52s (± 0.48%)2.52s (± 0.23%)+0.00s (+ 0.16%)2.51s2.53s
Bind Time0.87s (± 0.46%)0.87s (± 0.89%)0.00s ( 0.00%)0.86s0.89s
Check Time5.63s (± 0.43%)5.60s (± 0.48%)-0.03s (- 0.46%)5.56s5.68s
Emit Time6.35s (± 0.76%)6.35s (± 1.14%)+0.01s (+ 0.09%)6.16s6.47s
Total Time15.36s (± 0.33%)15.34s (± 0.47%)-0.02s (- 0.11%)15.21s15.53s
Monaco - node (v8.9.0, x64)
Memory used358,531k (± 0.01%)358,537k (± 0.01%)+6k (+ 0.00%)358,423k358,598k
Parse Time1.93s (± 0.30%)1.93s (± 0.40%)+0.00s (+ 0.21%)1.91s1.95s
Bind Time0.91s (± 0.54%)0.91s (± 0.49%)+0.01s (+ 0.66%)0.90s0.92s
Check Time5.65s (± 0.65%)5.71s (± 0.53%)+0.06s (+ 1.11%)5.65s5.78s
Emit Time3.40s (± 1.24%)3.43s (± 0.40%)+0.03s (+ 1.03%)3.41s3.46s
Total Time11.88s (± 0.46%)11.99s (± 0.31%)+0.12s (+ 0.97%)11.93s12.09s
TFS - node (v8.9.0, x64)
Memory used310,349k (± 0.01%)310,376k (± 0.01%)+27k (+ 0.01%)310,305k310,496k
Parse Time1.57s (± 0.43%)1.57s (± 0.37%)-0.01s (- 0.44%)1.56s1.58s
Bind Time0.69s (± 0.65%)0.69s (± 0.81%)0.00s ( 0.00%)0.68s0.70s
Check Time5.33s (± 0.56%)5.37s (± 0.44%)+0.04s (+ 0.83%)5.31s5.41s
Emit Time2.97s (± 0.59%)2.98s (± 0.97%)+0.01s (+ 0.27%)2.91s3.05s
Total Time10.56s (± 0.27%)10.61s (± 0.41%)+0.04s (+ 0.40%)10.52s10.70s
material-ui - node (v8.9.0, x64)
Memory used496,232k (± 0.01%)496,285k (± 0.01%)+53k (+ 0.01%)496,125k496,386k
Parse Time2.50s (± 0.48%)2.51s (± 0.88%)+0.01s (+ 0.44%)2.47s2.58s
Bind Time0.82s (± 1.69%)0.82s (± 1.41%)-0.00s (- 0.24%)0.79s0.84s
Check Time18.14s (± 0.86%)18.21s (± 0.89%)+0.07s (+ 0.39%)17.97s18.73s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time21.45s (± 0.73%)21.53s (± 0.80%)+0.08s (+ 0.37%)21.23s22.05s
Angular - node (v8.9.0, x86)
Memory used198,978k (± 0.02%)198,975k (± 0.03%)-3k (- 0.00%)198,803k199,118k
Parse Time2.44s (± 0.55%)2.44s (± 0.64%)-0.00s (- 0.04%)2.41s2.47s
Bind Time1.02s (± 0.87%)1.02s (± 0.75%)-0.01s (- 0.78%)1.00s1.03s
Check Time5.09s (± 0.58%)5.06s (± 0.53%)-0.02s (- 0.47%)5.01s5.13s
Emit Time6.12s (± 0.82%)6.12s (± 0.65%)+0.00s (+ 0.02%)6.05s6.20s
Total Time14.67s (± 0.52%)14.64s (± 0.49%)-0.03s (- 0.17%)14.50s14.81s
Monaco - node (v8.9.0, x86)
Memory used203,062k (± 0.02%)203,089k (± 0.02%)+27k (+ 0.01%)203,022k203,150k
Parse Time1.99s (± 0.76%)1.98s (± 0.94%)-0.01s (- 0.45%)1.96s2.04s
Bind Time0.72s (± 0.81%)0.72s (± 1.58%)+0.01s (+ 0.70%)0.71s0.76s
Check Time5.77s (± 0.60%)5.81s (± 0.59%)+0.04s (+ 0.73%)5.73s5.90s
Emit Time2.76s (± 1.39%)2.75s (± 0.85%)-0.01s (- 0.33%)2.71s2.81s
Total Time11.24s (± 0.32%)11.27s (± 0.49%)+0.03s (+ 0.23%)11.19s11.41s
TFS - node (v8.9.0, x86)
Memory used177,488k (± 0.02%)177,511k (± 0.02%)+24k (+ 0.01%)177,449k177,568k
Parse Time1.61s (± 0.72%)1.61s (± 1.03%)+0.00s (+ 0.19%)1.58s1.65s
Bind Time0.66s (± 0.76%)0.66s (± 1.04%)+0.00s (+ 0.15%)0.65s0.68s
Check Time4.90s (± 0.63%)4.87s (± 0.38%)-0.04s (- 0.73%)4.82s4.91s
Emit Time2.85s (± 0.73%)2.84s (± 0.80%)-0.01s (- 0.39%)2.77s2.88s
Total Time10.01s (± 0.42%)9.97s (± 0.50%)-0.04s (- 0.44%)9.85s10.06s
material-ui - node (v8.9.0, x86)
Memory used279,352k (± 0.02%)279,381k (± 0.02%)+29k (+ 0.01%)279,263k279,484k
Parse Time2.55s (± 0.65%)2.56s (± 0.37%)+0.00s (+ 0.08%)2.54s2.58s
Bind Time0.81s (± 6.62%)0.81s (± 6.31%)-0.00s (- 0.37%)0.70s0.89s
Check Time16.55s (± 0.40%)16.57s (± 1.00%)+0.02s (+ 0.11%)16.25s16.92s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time19.91s (± 0.45%)19.93s (± 0.78%)+0.02s (+ 0.10%)19.64s20.34s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
BenchmarkNameIterations
Current4154410
Baselinemaster10

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished andfailed. I've opened aPR with the baseline diff from master.

@ahejlsberg
Copy link
MemberAuthor

All tests look clean and perf appears unaffected.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedNov 20, 2020•
edited
Loading

Heya@DanielRosenwasser, I've started to run the tarball bundle task on this PR at77722bb. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedNov 20, 2020•
edited
Loading

Hey@DanielRosenwasser, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/89656/artifacts?artifactName=tgz&fileId=5EF2D96AB8BB9E4EB9303942A61DEA8C51BE11AC597FD4CE8F3C9ACCEA28D90E02&fileName=/typescript-4.2.0-insiders.20201120.tgz"    }}

and then runningnpm install.

There is also a playgroundfor this build.

Copy link
Member

@weswighamweswigham left a comment•
edited
Loading

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 onlyactually 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.

ExE-Boss reacted with thumbs up emojiethanresnick and Igorbek reacted with heart emoji
~~
!!! 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 requiredintype '[number, string, boolean, boolean, ...boolean[]]'.
!!! error TS2322:Source provides no match for required element at position 3intarget.
Copy link
Member

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

Suggested change
!!! error TS2322:Sourceprovides no match for required element at position 3 intarget.
!!! error TS2322:Type '[number, string, boolean, ...boolean[]]'provides no match for required element at position 3 intype '[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.

DanielRosenwasser reacted with thumbs up emoji

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.

typeTup3<Textendsunknown[],Uextendsunknown[],Vextendsunknown[]>=[...T, ...U, ...V];

typeV20=Tup3<[number],string[],[number]>;// [number, ...string[], number]
typeV21=Tup3<[number],[string?],[boolean]>;// [number, string | undefined, boolean]
Copy link
Member

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.


typeV20=Tup3<[number],string[],[number]>;// [number, ...string[], number]
typeV21=Tup3<[number],[string?],[boolean]>;// [number, string | undefined, boolean]
typeV22=Tup3<[number],string[],boolean[]>;// [number, (string | boolean)[]]
Copy link
Member

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.

typeV20=Tup3<[number],string[],[number]>;// [number, ...string[], number]
typeV21=Tup3<[number],[string?],[boolean]>;// [number, string | undefined, boolean]
typeV22=Tup3<[number],string[],boolean[]>;// [number, (string | boolean)[]]
typeV23=Tup3<[number],string[],[boolean?]>;// [number, (string | boolean | undefined)[]]
Copy link
Member

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.

typeV22=Tup3<[number],string[],boolean[]>;// [number, (string | boolean)[]]
typeV23=Tup3<[number],string[],[boolean?]>;// [number, (string | boolean | undefined)[]]
typeV24=Tup3<[number],[boolean?],string[]>;// [number, boolean?, ...string[]]
typeV25=Tup3<string[],number[],boolean[]>;// (string | number | boolean)[]
Copy link
Member

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 ismuch,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, butnot[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.

typeV23=Tup3<[number],string[],[boolean?]>;// [number, (string | boolean | undefined)[]]
typeV24=Tup3<[number],[boolean?],string[]>;// [number, boolean?, ...string[]]
typeV25=Tup3<string[],number[],boolean[]>;// (string | number | boolean)[]
typeV26=Tup3<string[],number[],[boolean]>;// [...(string | number)[], boolean]
Copy link
Member

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.

typeV24=Tup3<[number],[boolean?],string[]>;// [number, boolean?, ...string[]]
typeV25=Tup3<string[],number[],boolean[]>;// (string | number | boolean)[]
typeV26=Tup3<string[],number[],[boolean]>;// [...(string | number)[], boolean]
typeV27=Tup3<[number?],[string],[boolean?]>;// [number | undefined, string, boolean?]
Copy link
Member

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.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commentedDec 7, 2020•
edited
Loading

@weswigham from your comments, it sounds like you want to loosen the restriction in place:

Tuple type layouts are now governed by these rules:

  • An optional element cannot precede a required element or follow a rest element.
  • Multiple rest elements are not permitted.

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

typeFoo=[string,string?,number];

into something like

typeFoo=[string,number]|[string,string,number];

though not the suggested

typeFoo=[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.

TylorS reacted with thumbs up emoji

@weswigham
Copy link
Member

it sounds like you want to loosen the restriction in place:

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.

How often do we leverage the distinction between optional and potentially undefined when we have the property itself?

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 ofundefined in the type is potentially needed for consistency.

For multiple rest elements...

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.

TylorS and Igorbek reacted with thumbs up emoji

@ahejlsbergahejlsberg merged commit9b17186 intomasterJan 5, 2021
@ahejlsbergahejlsberg deleted the fix39595 branchJanuary 5, 2021 01:13
Zzzen pushed a commit to Zzzen/TypeScript that referenced this pull requestJan 16, 2021
* 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
@microsoftmicrosoft locked asresolvedand limited conversation to collaboratorsOct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@weswighamweswighamweswigham left review comments

@DanielRosenwasserDanielRosenwasserAwaiting requested review from DanielRosenwasser

@RyanCavanaughRyanCavanaughAwaiting requested review from RyanCavanaugh

@sandersnsandersnAwaiting requested review from sandersn

Assignees

@ahejlsbergahejlsberg

Labels

Author: TeamFor Milestone BugPRs that fix a bug with a specific milestone

Projects

Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support inferring rest args that are not at the end of the function's args

5 participants

@ahejlsberg@typescript-bot@DanielRosenwasser@weswigham
[8]ページ先頭
©2009-2025 Movatter.jp