Conversation
jakebailey
requested review from
DanielRosenwasser,
RyanCavanaugh,
ahejlsberg and
Copilot
|
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.
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
outerReturnMapperinfrastructure 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. |
1 similar comment
4 participants
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.