-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Only resolve source return type when actually needed during inference #58650
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
Conversation
# Conflicts: # tests/baselines/reference/circularReferenceInReturnType.types
@typescript-bot test it |
Hey @ahejlsberg, the results of running the DT tests are ready. Everything looks the same! |
@ahejlsberg Here are the results of running the user tests comparing Something interesting changed - please have a look. Details
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@@ -3,7 +3,7 @@ | |||
=== Performance Stats === | |||
Assignability cache: 10,000 | |||
Type Count: 50,000 | |||
Instantiation count: 250,000 | |||
Instantiation count: 100,000 |
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.
Nice!
@@ -28,7 +28,7 @@ flatArrayNoExcessiveStackDepth.ts(20,5): error TS2322: Type 'FlatArray<Arr, any> | |||
~ | |||
!!! error TS2322: Type 'FlatArray<Arr, any>' is not assignable to type 'FlatArray<Arr, D>'. | |||
!!! error TS2322: Type 'Arr' is not assignable to type 'FlatArray<Arr, D>'. | |||
!!! error TS2322: Type 'Arr' is not assignable to type '(Arr extends readonly (infer InnerArr)[] ? FlatArray<InnerArr, [-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20][D]> : Arr) & Arr'. | |||
!!! error TS2322: Type 'Arr' is not assignable to type 'Arr & (Arr extends readonly (infer InnerArr)[] ? FlatArray<InnerArr, [-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20][D]> : Arr)'. |
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 guessing that because a return type was not eagerly calculated at some step, one of these types was created later than the other.
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.
Though we don't sort type IDs in intersections, so that's not it.
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 sure it's a case of a distributing over a union type and turning it into an intersection type in a write-position. We do that in several places in type simplification and type relations. For example, an indexed access type T[K]
, where K
is a union type, occurring in a write-position.
@typescript-bot pack this |
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 and an npm module you can use via |
@ahejlsberg Here are the results of running the top 400 repos comparing Something interesting changed - please have a look. Details
|
The Webpack change is that there are now two implicit |
Looks to me like we now get a bit deeper in type resolution before detecting the circularity in here, and thus we report an error in one more place (because the resolution stack is one level deeper). This actually improves the error reporting because it makes it easier to see what causes the circularity. |
The new errors in the tinacms repo result from the absence of circularities that previously caused arrow functions to have implicit |
I think this is ready to merge. Performance is unaffected and the new test suite errors in are all desirable and correct. |
Fixes #58616. Also fixes an unnecessary circularity in one of our existing tests.