Fix translation error that disabled union discrimination optimization #1757
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a translation error that disabled union discrimination optimization. The issue was caused by incorrect condition checking and an extra closing brace that broke the control flow in the type mapping logic.
- Consolidated discriminant validation logic in
mapTypesByKeyProperty - Fixed condition inversion in
getMatchingUnionConstituentForObjectLiteral - Removed extraneous closing brace that was breaking function structure
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/checker/relater.go |
Consolidated discriminant nil and literal type checks, removed extra closing brace |
internal/checker/checker.go |
Fixed inverted condition for key property name validation |
testdata/tests/cases/compiler/missingDiscriminants2.ts |
Added test case for discriminated unions with >10 cases to verify the optimization works |
testdata/baselines/reference/compiler/missingDiscriminants2.* |
Generated baseline files for the new test case |
| } | ||
| if !duplicate { | ||
| count++ | ||
| if discriminant == nil || !isLiteralType(discriminant) { |
There was a problem hiding this comment.
Doesn't this behave differently from the original code? The original code would still continue the loop of the discriminate was undefined.
There was a problem hiding this comment.
Yes, but the exit is intended. We only want to build a map when all constituents have a discriminant. When some constituents lack a discriminant, we want to skip the map optimization and go through the regular discriminateTypeByDiscriminableItems code path.
|
Hm, microsoft/TypeScript#62501 (comment) shows a break in DT |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Implements what I describe here.
Fixes #1727.