-
Notifications
You must be signed in to change notification settings - Fork 13k
Defer checking subexpressions of binary expressions with known boolean result type #60865
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?
Defer checking subexpressions of binary expressions with known boolean result type #60865
Conversation
src/compiler/checker.ts
Outdated
// Those boolean arguments wouldn't even have to be viable sources for type arguments being inferred. | ||
// | ||
// For those reasons, we defer obtaining the operand types here and checking the related errors. | ||
checkNodeDeferred(node); |
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.
This is really the core of the fix. The rest of the PR only moves code around (for the most part)
src/compiler/checker.ts
Outdated
state.skip = true; | ||
setLastResult(state, booleanType); | ||
break; | ||
case SyntaxKind.QuestionQuestionToken: |
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.
Out of curiosity (this may be a dumb question): Since SyntaxKind.EqualsToken
(which I assume refers to assignment) is handled above and SyntaxKind.QuestionQuestionToken
is handled here, should nullish coalescing assignment (SyntaxKind.QuestionQuestionEqualsToken
) also be handled here?
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.
It's a valid question. I think no adjustments have to be made here though. Only =
is permitted when the left side is a destructuring pattern. Other kinds of assignment~ operators in such scenarios result in a syntax error:
let a: number | undefined;
({ a } ??= { a: 10 }); // Uncaught SyntaxError: Invalid left-hand side in assignment
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 refactors the TypeScript compiler's handling of binary expressions with known boolean result types by removing a recently added mechanism in favor of the older checkDeferredNode
approach. The change aims to reduce redundant type checking errors, particularly in control flow analysis scenarios where binary expressions like comparisons have predetermined boolean types.
- Removes the
CheckMode.TypeOnly
flag mechanism introduced in a previous PR - Switches to using
checkDeferredNode
for binary expressions with boolean results - Adds specific handling for comparison and equality operators to defer type checking until later
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/compiler/types.ts |
Adds NullishCoalesceExpression interface for type safety |
src/compiler/checker.ts |
Major refactoring of binary expression checking, removes CheckMode.TypeOnly , adds deferred checking for boolean operators |
Test files | Adds new test cases for control flow inference and updates existing baselines |
fixes #60130
fixes #62279
This PR removes the mechanism introduced in #54380 in favor of using the older
checkDeferredNode
mechanism that allows the compiler to avoid an extra set of redundant errors.