Skip to content

Conversation

@ahejlsberg
Copy link
Member

@ahejlsbergahejlsberg commented Aug 11, 2020

With this PR we officially support recursive conditional types. For example:

// Awaiting promisestypeAwaited<T>=Textendsnull|undefined ? T : TextendsPromiseLike<infer U> ? Awaited<U> : T;typeP1=Awaited<Promise<string>>;// stringtypeP2=Awaited<Promise<Promise<string>>>;// stringtypeP3=Awaited<Promise<string|Promise<Promise<number>|undefined>>;// string | number | undefined

A bit of context... When conditional types were first introduced, we explicitly restricted them to be non-recursive, i.e. we made it an error for a conditional type to directly or indirectly reference itself. This restriction was put in place primarily as a safeguard against runaway infinite recursion which the compiler didn't handle well at the time. However, it turns out it was still possible to construct recursive conditionally resolved types by combining object types (which have deferred resolution of property types) and conditional types using indexed access types (see #14833 for the core idea). In spite of being cumbersome and non-intuitive, this trick has become commonplace in several libraries. Consequently, over time we have "hardened" the compiler against infinite recursion with depth limiters in relationships, type inference, type instantiation, constraint computation, and so on. We're now at a point where it seems reasonable support an intuitive way of writing recursive conditional types.

Some more examples:

// Flattening arraystypeFlatten<Textendsreadonlyunknown[]>=Textendsunknown[] ? _Flatten<T>[] : readonly_Flatten<T>[];type_Flatten<T>=Textendsreadonly(infer U)[] ? _Flatten<U> : T;typeInfiniteArray<T>=InfiniteArray<T>[];typeA1=Flatten<string[][][]>;// string[]typeA2=Flatten<string[][]|readonly(number[]|boolean[][])[]>;// string[] | readonly (number | boolean)[]typeA3=Flatten<InfiniteArray<string>>;typeA4=A3[0];// Infinite depth error// Repeating tuplestypeTupleOf<T,Nextendsnumber>=NextendsN ? numberextendsN ? T[] : _TupleOf<T,N,[]> : never;type_TupleOf<T,Nextendsnumber,Rextendsunknown[]>=R['length']extendsN ? R : _TupleOf<T,N,[T, ...R]>;typeT1=TupleOf<string,3>;// [string, string, string]typeT2=TupleOf<number,0|2|4>;// [] | [number, number] | [number, number, number, number]typeT3=TupleOf<number,number>;// number[]typeT4=TupleOf<number,100>;// Depth error

Note that this PR doesn't change the recursion depth limits that are already in place. For example, an error is reported on T4 above because its resolution exceeds the limit of 50 nested type instantiations.

The type inference streamlining contained in the PR fixes several issues with inference to recursive types. For example:

interfaceBox<T>{value: T};typeRecBox<T>=T|Box<RecBox<T>>;declarefunctionunbox<T>(box: RecBox<T>): TtypeT1=Box<string>;typeT2=Box<T1>;typeT3=Box<T2>;typeT4=Box<T3>;typeT5=Box<T4>;typeT6=Box<T5>;declareletb1: Box<Box<Box<Box<Box<Box<string>>>>>>;declareletb2: T6;unbox(b1);// stringunbox(b2);// string (previously T6)unbox({value: {value: {value: 5}}});// number (previously{value:{value: number }})

Previously, only the unbox(b1) call produced the expected type inference.

Fixes#26223.
Fixes#26980.
Fixes#37801.

@typescript-bottypescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Aug 11, 2020
@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 commented Aug 11, 2020

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 7c4d923. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 11, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 7c4d923. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 11, 2020

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 7c4d923. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 11, 2020

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 7c4d923. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..40002

Metricmaster40002DeltaBestWorst
Angular - node (v10.16.3, x64)
Memory used343,622k (± 0.02%)343,145k (± 0.03%)-477k (- 0.14%)342,941k343,375k
Parse Time2.02s (± 0.70%)2.00s (± 0.59%)-0.02s (- 0.74%)1.98s2.03s
Bind Time0.82s (± 0.82%)0.82s (± 0.98%)+0.00s (+ 0.12%)0.81s0.84s
Check Time4.77s (± 0.90%)4.78s (± 0.62%)+0.01s (+ 0.21%)4.70s4.83s
Emit Time5.19s (± 0.87%)5.21s (± 0.71%)+0.01s (+ 0.29%)5.14s5.34s
Total Time12.80s (± 0.71%)12.81s (± 0.52%)+0.01s (+ 0.08%)12.70s13.02s
Monaco - node (v10.16.3, x64)
Memory used339,127k (± 0.02%)339,168k (± 0.02%)+41k (+ 0.01%)339,004k339,281k
Parse Time1.59s (± 0.84%)1.57s (± 0.49%)-0.03s (- 1.88%)1.55s1.58s
Bind Time0.72s (± 0.66%)0.71s (± 0.67%)-0.00s (- 0.56%)0.70s0.72s
Check Time4.94s (± 0.62%)4.89s (± 0.52%)-0.05s (- 1.01%)4.80s4.92s
Emit Time2.77s (± 1.05%)2.74s (± 0.76%)-0.03s (- 1.05%)2.71s2.78s
Total Time10.02s (± 0.58%)9.90s (± 0.30%)-0.12s (- 1.16%)9.83s9.95s
TFS - node (v10.16.3, x64)
Memory used0k0k0k ( NaN%)0k0k
Parse Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Bind Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Check Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Emit Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Total Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
material-ui - node (v10.16.3, x64)
Memory used458,780k (± 0.01%)460,209k (± 0.01%)+1,428k (+ 0.31%)460,011k460,312k
Parse Time2.05s (± 0.46%)2.05s (± 0.52%)-0.01s (- 0.39%)2.03s2.07s
Bind Time0.66s (± 1.46%)0.65s (± 1.25%)-0.01s (- 1.66%)0.64s0.67s
Check Time12.97s (± 0.66%)13.21s (± 0.51%)+0.24s (+ 1.84%)13.08s13.41s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time15.68s (± 0.54%)15.90s (± 0.47%)+0.22s (+ 1.40%)15.75s16.13s
Angular - node (v12.1.0, x64)
Memory used320,898k (± 0.02%)320,378k (± 0.02%)-520k (- 0.16%)320,232k320,453k
Parse Time2.01s (± 0.86%)1.99s (± 0.73%)-0.02s (- 0.90%)1.95s2.03s
Bind Time0.80s (± 1.03%)0.80s (± 0.74%)+0.00s (+ 0.00%)0.79s0.82s
Check Time4.66s (± 0.49%)4.65s (± 0.64%)-0.01s (- 0.24%)4.59s4.70s
Emit Time5.38s (± 0.50%)5.35s (± 0.49%)-0.03s (- 0.61%)5.29s5.40s
Total Time12.85s (± 0.41%)12.79s (± 0.44%)-0.06s (- 0.46%)12.69s12.90s
Monaco - node (v12.1.0, x64)
Memory used321,569k (± 0.03%)321,560k (± 0.03%)-10k (- 0.00%)321,395k321,734k
Parse Time1.55s (± 0.89%)1.54s (± 0.66%)-0.01s (- 0.65%)1.51s1.56s
Bind Time0.69s (± 1.05%)0.69s (± 0.83%)-0.00s (- 0.29%)0.68s0.70s
Check Time4.74s (± 0.32%)4.69s (± 0.46%)-0.05s (- 0.99%)4.66s4.76s
Emit Time2.82s (± 0.50%)2.79s (± 0.86%)-0.02s (- 0.85%)2.75s2.86s
Total Time9.79s (± 0.23%)9.71s (± 0.36%)-0.08s (- 0.84%)9.63s9.79s
TFS - node (v12.1.0, x64)
Memory used0k0k0k ( NaN%)0k0k
Parse Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Bind Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Check Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Emit Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Total Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
material-ui - node (v12.1.0, x64)
Memory used437,163k (± 0.01%)438,348k (± 0.07%)+1,185k (+ 0.27%)437,478k438,652k
Parse Time2.04s (± 0.49%)2.02s (± 0.64%)-0.03s (- 1.27%)1.97s2.03s
Bind Time0.64s (± 0.57%)0.62s (± 0.58%)-0.01s (- 1.89%)0.62s0.63s
Check Time11.82s (± 0.48%)11.83s (± 1.13%)+0.01s (+ 0.06%)11.66s12.28s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time14.50s (± 0.34%)14.47s (± 0.95%)-0.03s (- 0.21%)14.31s14.93s
Angular - node (v8.9.0, x64)
Memory used340,207k (± 0.02%)339,812k (± 0.02%)-395k (- 0.12%)339,619k339,967k
Parse Time2.55s (± 0.68%)2.53s (± 0.42%)-0.01s (- 0.47%)2.51s2.56s
Bind Time0.86s (± 0.52%)0.85s (± 0.68%)-0.01s (- 0.93%)0.84s0.86s
Check Time5.42s (± 0.45%)5.40s (± 0.59%)-0.02s (- 0.44%)5.34s5.46s
Emit Time5.98s (± 1.57%)5.92s (± 0.85%)-0.07s (- 1.12%)5.76s5.99s
Total Time14.81s (± 0.73%)14.70s (± 0.53%)-0.12s (- 0.80%)14.46s14.81s
Monaco - node (v8.9.0, x64)
Memory used340,558k (± 0.02%)340,543k (± 0.02%)-15k (- 0.00%)340,392k340,687k
Parse Time1.88s (± 0.43%)1.87s (± 0.69%)-0.01s (- 0.37%)1.85s1.91s
Bind Time0.89s (± 0.69%)0.89s (± 0.73%)-0.00s (- 0.34%)0.88s0.91s
Check Time5.46s (± 0.57%)5.40s (± 0.38%)-0.06s (- 1.08%)5.36s5.44s
Emit Time3.24s (± 1.02%)3.24s (± 0.98%)-0.00s (- 0.09%)3.18s3.32s
Total Time11.46s (± 0.46%)11.40s (± 0.45%)-0.07s (- 0.58%)11.28s11.52s
TFS - node (v8.9.0, x64)
Memory used0k0k0k ( NaN%)0k0k
Parse Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Bind Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Check Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Emit Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Total Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
material-ui - node (v8.9.0, x64)
Memory used463,031k (± 0.01%)464,543k (± 0.01%)+1,512k (+ 0.33%)464,410k464,658k
Parse Time2.41s (± 0.71%)2.39s (± 0.57%)-0.02s (- 1.00%)2.35s2.42s
Bind Time0.79s (± 1.61%)0.79s (± 2.04%)-0.01s (- 0.76%)0.76s0.83s
Check Time17.26s (± 1.01%)17.75s (± 0.83%)+0.49s (+ 2.86%)17.45s18.11s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time20.46s (± 0.88%)20.93s (± 0.70%)+0.47s (+ 2.28%)20.65s21.27s
Angular - node (v8.9.0, x86)
Memory used195,206k (± 0.04%)195,040k (± 0.03%)-166k (- 0.08%)194,913k195,193k
Parse Time2.46s (± 0.43%)2.46s (± 0.88%)-0.00s (- 0.12%)2.42s2.50s
Bind Time0.99s (± 1.06%)0.99s (± 1.34%)+0.00s (+ 0.40%)0.96s1.02s
Check Time4.87s (± 0.75%)4.86s (± 0.72%)-0.01s (- 0.25%)4.79s4.93s
Emit Time5.97s (± 1.52%)5.95s (± 1.30%)-0.02s (- 0.39%)5.72s6.10s
Total Time14.30s (± 0.76%)14.27s (± 0.74%)-0.03s (- 0.23%)14.04s14.51s
Monaco - node (v8.9.0, x86)
Memory used193,549k (± 0.03%)193,538k (± 0.02%)-11k (- 0.01%)193,481k193,629k
Parse Time1.92s (± 0.83%)1.92s (± 1.46%)+0.01s (+ 0.42%)1.87s1.99s
Bind Time0.70s (± 0.43%)0.71s (± 1.28%)+0.01s (+ 1.58%)0.70s0.74s
Check Time5.50s (± 1.49%)5.57s (± 0.35%)+0.07s (+ 1.22%)5.51s5.61s
Emit Time2.71s (± 3.10%)2.66s (± 0.98%)-0.04s (- 1.55%)2.63s2.76s
Total Time10.82s (± 0.66%)10.86s (± 0.37%)+0.04s (+ 0.36%)10.78s10.97s
TFS - node (v8.9.0, x86)
Memory used0k0k0k ( NaN%)0k0k
Parse Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Bind Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Check Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Emit Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
Total Time0.00s0.00s0.00s ( NaN%)0.00s0.00s
material-ui - node (v8.9.0, x86)
Memory used262,269k (± 0.02%)263,018k (± 0.02%)+749k (+ 0.29%)262,896k263,212k
Parse Time2.46s (± 0.61%)2.43s (± 0.41%)-0.03s (- 1.06%)2.41s2.46s
Bind Time0.68s (± 1.38%)0.68s (± 1.70%)-0.00s (- 0.44%)0.66s0.71s
Check Time15.74s (± 0.86%)16.25s (± 0.53%)+0.51s (+ 3.25%)16.03s16.43s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time18.88s (± 0.68%)19.36s (± 0.44%)+0.48s (+ 2.55%)19.20s19.54s
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
Current4000210
Baselinemaster10

}

function invokeOnce(source: Type, target: Type, action: (source: Type, target: Type) => void){
function invokeWithDepthLimit(source: Type, target: Type, action: (source: Type, target: Type) => void){
Copy link
Member

Choose a reason for hiding this comment

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

This is pretty much the inference equivalent of recursiveTypeRelatedTo at this point.

}
if (type.flags & TypeFlags.Conditional){
// The root object represents the origin of the conditional type
return (type as ConditionalType).root;
Copy link
Member

Choose a reason for hiding this comment

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

Huh, and this doesn't affect the user baselines or DT?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

No. Previously conditional types couldn't be recursive, so they weren't included here. But for that same reason we have no tests that could be affected by this.

@treybrisbane
Copy link

Awesome to finally see this! 😀

@DanielRosenwasser
Copy link
Member

@ahejlsberg maybe something worthwhile is to build another PR on top of this to see if changing the definition of FlatArray would impact real-world code.

@ahejlsberg
Copy link
MemberAuthor

The tests revealed OOMs in a few projects due to the switch to use isDeeplyNestedType for recursion tracking in type inference (which permits up to five levels of recursion). With the latest commits I have reverted to the previous scheme of terminating after just one level of recursion, but with the added twist that we track both the source and target sides (similarly to recursiveTypeRelatedTo) and terminate only when both have a circularity.

Intuitively, in inference we want to terminate when we encounter a duplicate attempt to infer from source and target types with the same origin, so getRecursionIdentity needs to get us as close as possible to the AST node that caused the type instantiation. Because most type instantiations are interned and shared (and thus have no reference to their originating AST node), this isn't always possible. For example, when inferring from Box2<Box2<string>> to Box1<Box1<T>>, where Box1 and Box2 are unique but structurally identical types, we end up with the same recursion identity for each Box1 and Box2 reference, and therefore terminate inference prematurely. This is a known problem (i.e. not new to this PR) and something we should continue to think about. A brute force way to work around the problem is to allow multiple levels of recursion, but that only works up to some level of nested and, as illustrated by the test failures, generates way too much work in general.

@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 commented Aug 13, 2020

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at fed0e8c. You can monitor the build here.

@AlCalzone
Copy link
Contributor

@ahejlsberg According to your example in the OP, this also fixes #26223 😊

@ahejlsberg
Copy link
MemberAuthor

Test runs all look clean. Slight regression in check time for material-ui, but it's worth it for the added precision in type inference.

@ahejlsbergahejlsberg merged commit cd30534 into masterAug 16, 2020
@ahejlsbergahejlsberg deleted the recursiveConditionalTypes branch August 16, 2020 01:26
@strelga
Copy link

@ahejlsberg@rbuckton

Does this change mean we no longer need hacks like awaited keyword to handle the recursive nature of Promise?

@tjjfvi
Copy link
Contributor

Can we get a playground for this PR? I'd like to play around with the new options this gives us.

@levenleven
Copy link

@tjjfvi you can choose "Nightly" version
image

@lusess123
Copy link

lusess123 commented Oct 4, 2021

typeT4=TupleOf<number,100>;// Depth error

why the max number is 43 ? not 48 ? not 98 ?

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

11 participants

@ahejlsberg@typescript-bot@treybrisbane@DanielRosenwasser@AlCalzone@strelga@tjjfvi@levenleven@lusess123@weswigham