Skip to content

Fix narrowing of destructured discriminated union parameters with defaults#63505

Open
wolfe8105 wants to merge 1 commit into
microsoft:mainfrom
wolfe8105:fix/50139-discriminated-union-destructure-defaults
Open

Fix narrowing of destructured discriminated union parameters with defaults#63505
wolfe8105 wants to merge 1 commit into
microsoft:mainfrom
wolfe8105:fix/50139-discriminated-union-destructure-defaults

Conversation

@wolfe8105
Copy link
Copy Markdown

Fixes #50139 — first PR here, happy to adjust style/scope as needed.

src/compiler/checker.ts had two predicates that skipped narrowing for destructured bindings with defaults:

  • getCandidateDiscriminantPropertyAccess
  • getNarrowedTypeOfSymbol

Both guarded with !declaration.initializer. Removed that clause from both. Narrowing now works through defaulted bindings.

Declaration-time assignability of the default is checked by a separate pipeline, so foreign-type defaults still error at the binding.

Regression test added. npx hereby runtests shows no existing baselines change.


Copilot AI review requested due to automatic review settings May 25, 2026 15:21
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog May 25, 2026
@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug labels May 25, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a compiler regression test for destructuring defaults in discriminated unions (issue #50139) and updates checker logic so narrowing works when binding elements/parameters have default initializers.

Changes:

  • Add a new compiler test covering: (1) repro with defaults, (2) control without defaults, (3) soundness error case.
  • Update checker.ts control-flow/narrowing logic to consider binding elements/parameters even when they have default initializers.
  • Add new baseline outputs (.types, .symbols, .js, .errors.txt) for the test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/cases/compiler/discriminatedUnionDestructuringWithDefaults.ts New regression test for narrowing behavior with destructuring defaults in discriminated unions
tests/baselines/reference/discriminatedUnionDestructuringWithDefaults.types Baseline types output for the new test
tests/baselines/reference/discriminatedUnionDestructuringWithDefaults.symbols Baseline symbols output for the new test
tests/baselines/reference/discriminatedUnionDestructuringWithDefaults.js Baseline JS emit output for the new test
tests/baselines/reference/discriminatedUnionDestructuringWithDefaults.errors.txt Baseline expected error output for the soundness case
src/compiler/checker.ts Adjust narrowing/reference logic to no longer exclude binding elements/parameters with initializers
Comments suppressed due to low confidence (1)

src/compiler/checker.ts:1

  • This condition used to explicitly exclude binding elements with an initializer; removing that guard is subtle and easy to accidentally revert later. Consider adding a brief comment here explaining that binding element default initializers are intentionally included so discriminated-union narrowing works for destructured bindings with defaults (as covered by the new test).

@wolfe8105
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@MartinJohns
Copy link
Copy Markdown
Contributor

@wolfe8105 You should read this: #62963

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Backlog Bug PRs that fix a backlog bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

Discriminated union parameter destructuring doesn't work if the fields have defaults

4 participants