Narrow destructured discriminated unions when the discriminant has a default#63563
Open
DukeDeSouth wants to merge 1 commit into
Open
Conversation
A default on a required discriminant binding no longer disables discriminant narrowing of the sibling bindings. The default is ignored for narrowing only when it can never apply (the property is present and non-undefined in every union constituent), so optional or possibly-undefined discriminants keep their previous sound behavior. Fixes microsoft#50139
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
1 similar comment
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test and compiler fix to ensure discriminant narrowing still works when a required discriminant is destructured with a default initializer, while preserving existing soundness boundaries for optional/possibly-undefined discriminants.
Changes:
- Added a new conformance test covering narrowing behavior with required discriminants that have destructuring defaults (plus negative/soundness cases).
- Updated
checker.tsto allow dependent narrowing through binding elements with defaults only when the default can never apply (property always present and non-undefinedacross the declared union). - Added corresponding reference baselines (
.types,.symbols,.errors.txt) for the new test.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cases/conformance/controlFlow/dependentDestructuredVariablesWithDefaults.ts | New regression test validating narrowing with required discriminants + defaults and guarding soundness boundaries. |
| tests/baselines/reference/dependentDestructuredVariablesWithDefaults.types | Baseline updated to reflect the new test’s expected type/narrowing results. |
| tests/baselines/reference/dependentDestructuredVariablesWithDefaults.symbols | Baseline updated to reflect expected symbol binding results for the new test. |
| tests/baselines/reference/dependentDestructuredVariablesWithDefaults.errors.txt | Baseline updated to capture expected errors for unsound narrowing cases. |
| src/compiler/checker.ts | Compiler change enabling dependent narrowing through destructured defaults when the default is provably never used. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A default on the discriminant of a destructured union disables narrowing for the sibling bindings -
{ isText = false, children }: Propsleaveschildrenasstring | numberin both branches. It now still narrows when the default can never fire (the property is required and non-undefined across the union). Optional or possibly-undefined discriminants, and siblings with their own default, stay unchanged. Added a conformance test covering both.Fixes #50139