Recursive conditional types #40002
Recursive conditional types #40002
Conversation
typescript-bot
added
the
For Milestone Bug
ahejlsberg
requested review from
DanielRosenwasser,
RyanCavanaugh and
weswigham
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 7c4d923. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 7c4d923. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 7c4d923. You can monitor the build here. |
|
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! |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@ahejlsberg Here they are:Comparison Report - master..40002
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
src/compiler/checker.ts
Outdated
| @@ -19533,7 +19494,7 @@ namespace ts { | |||
| priority = savePriority; | |||
| } | |||
|
|
|||
| function invokeOnce(source: Type, target: Type, action: (source: Type, target: Type) => void) { | |||
| function invokeWithDepthLimit(source: Type, target: Type, action: (source: Type, target: Type) => void) { | |||
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Huh, and this doesn't affect the user baselines or DT?
There was a problem hiding this comment.
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.
|
Awesome to finally see this! 😀 |
|
@ahejlsberg maybe something worthwhile is to build another PR on top of this to see if changing the definition of |
|
The tests revealed OOMs in a few projects due to the switch to use 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 |
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at fed0e8c. You can monitor the build here. |
| if (sourceIdentity) (sourceStack || (sourceStack = [])).push(sourceIdentity); | ||
| if (targetIdentity) (targetStack || (targetStack = [])).push(targetIdentity); |
There was a problem hiding this comment.
| if (sourceIdentity) (sourceStack || (sourceStack = [])).push(sourceIdentity); | |
| if (targetIdentity) (targetStack || (targetStack = [])).push(targetIdentity); | |
| if (sourceIdentity) (sourceStack ||= []).push(sourceIdentity); | |
| if (targetIdentity) (targetStack ||= []).push(targetIdentity); |
There was a problem hiding this comment.
I'm going to leave it for now. We have lots of occurrences of that pattern, so maybe another PR to clean them all up.
|
@ahejlsberg According to your example in the OP, this also fixes #26223 😊 |
|
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. |
|
Does this change mean we no longer need hacks like |
|
Can we get a playground for this PR? I'd like to play around with the new options this gives us. |
|
@tjjfvi you can choose "Nightly" version |
why the max number is 43 ? not 48 ? not 98 ? |
With this PR we officially support recursive conditional types. For example:
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:
Note that this PR doesn't change the recursion depth limits that are already in place. For example, an error is reported on
T4above 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:
Previously, only the
unbox(b1)call produced the expected type inference.Fixes #26223.
Fixes #26980.
Fixes #37801.