-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Combine inferences from distributive conditional types #62093
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?
Combine inferences from distributive conditional types #62093
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 implements a fix for TypeScript issue #62079 by combining inferences from distributive conditional types. The change modifies the type inference system to properly handle parameters that use distributive conditional types, ensuring that inferences from multiple union member distributions are correctly combined.
- Adds a new
DistributiveConditional
inference priority flag to track distributive conditional type inferences - Modifies inference priority logic to combine distributive conditional inferences with other priorities
- Updates the conditional type inference process to mark distributive conditional types with the new priority
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/compiler/types.ts |
Adds DistributiveConditional inference priority flag and includes it in combination logic |
src/compiler/checker.ts |
Implements distributive conditional inference tracking and priority combination logic |
tests/cases/compiler/genericFunctionParametersConditionalType1.ts |
Test case demonstrating the fix for distributive conditional type parameter inference |
tests/baselines/reference/genericFunctionParametersConditionalType1.types |
Expected type output showing proper union distribution in parameter types |
tests/baselines/reference/genericFunctionParametersConditionalType1.symbols |
Expected symbol resolution output for the test case |
Comments suppressed due to low confidence (1)
@@ -26784,13 +26784,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
if (candidate === blockedStringType) { | |||
return; | |||
} | |||
if (inference.priority === undefined || priority < inference.priority) { | |||
const combinedPriority = priority | (inference.individualPriority || InferencePriority.None); |
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 don't quite like how this turned out - I probably should move this thing to a separate InferenceInfo
property (similar to topLevel
) instead of shoving it into .priority
.
That said, those changes would be cosmetic and wouldn't really change the idea behind this fix. In the meantime, I'd love to see what user/top tests have to say about this (cc @jakebailey )
@typescript-bot test it |
Hey @RyanCavanaugh, the results of running the DT tests are ready. Everything looks the same! |
@RyanCavanaugh Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
@RyanCavanaugh Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@RyanCavanaugh Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
fixes #62079