-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Revert PR 61668 #62158
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
base: main
Are you sure you want to change the base?
Revert PR 61668 #62158
Conversation
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
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.
Pull Request Overview
This PR reverts #61668, a change that appears to have introduced modifications to TypeScript's type inference system. The revert addresses issue #62071 while reopening #59937 and #61667, suggesting the original changes caused unexpected behavior in type inference.
Key changes:
- Reverts modifications to type inference and signature handling in the TypeScript compiler
- Removes the
outerReturnMapper
infrastructure that was added for handling nested inference contexts - Restores previous type variable instantiation behavior for single signature types
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/cases/compiler/inferencePromiseArrays1.ts | Adds new test case for Promise array inference scenarios |
src/compiler/types.ts | Reverts type system changes including SingleSignatureType interface and outerReturnMapper removal |
src/compiler/checker.ts | Reverts major inference engine changes and signature instantiation logic |
tests/baselines/reference/*.types | Updates type inference baselines reflecting the reverted behavior |
IMHO this is a just a change in inference. It broke things - sure. But some extra I looked into one of the breaks related to Promise.all and that was caused by the fact that a contextual type there is now instantiated based on the outer context mapper and that produced an array result. Previously the type would stay uninstantiated and thus it would provide a tuple context hint (because the contextual type stayed to be a homomorphic mapped type). But anyway, any change in inference can break something - it’s almost an unavoidable risk. |
This reverts #61668
Fixes #62071
Reopens #59937
Reopens #61667
But, it's not clear to me why this sort of thing was not caught in the extended tests, nor do I know if this is a good or bad break, or just differing inference. Posting the revert anyway for discussion.