Skip to content

Skip discriminant lookups for unions that cannot be narrowed - #64934

Open
Mateusz Burzyński (Andarist) wants to merge 2 commits into
microsoft:mainfrom
Andarist:experiment/discriminable-union-gate
Open

Mateusz Burzyński (Andarist) wants to merge 2 commits into
microsoft:mainfrom
Andarist:experiment/discriminable-union-gate

Conversation

@Andarist

@Andarist Mateusz Burzyński (Andarist) commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Just a potential minor optimization

project contextual calls contextual bails contextual names skipped relation calls relation bails relation names skipped
vscode 222,110 94,641 (43%) 488,900 11,337 1,078 3,265
xstate 110,649 69,190 (63%) 128,903 3,307 557 1,032
xstate-5 85,567 55,380 (65%) 105,169 2,068 451 858
arktype 13,641 1,920 (14%) 4,212 1,413 147 410
Compiler-Unions 2,618 1,547 (59%) 196,559 106 0 0

in the above table:
relation -> findMatchingDiscriminantType reached from excess property checking
names skipped -> upper bound of individual property names that would otherwise be passed through isDiscriminantProperty

Copilot AI balanced review requested due to automatic review settings October 23, 2026 10:42
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Oct 23, 2026
@typescript-automation typescript-automation Bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 23, 2026
@typescript-automation

Copy link
Copy Markdown
Contributor

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The contextual-typing early returns skip primitive filtering, suppressing deprecated-property suggestions for nullable object and JSX contextual types.

1 open issue
What changed in this PR

This PR aims to reduce compiler work by skipping discriminant lookups for unions with fewer than two non-primitive constituents.

Changes:

  • Adds a shared eligibility check for discriminant-based narrowing.
  • Applies early exits to type matching and contextual typing for object literals and JSX attributes.
File Description
tsc/​internal/​checker/​relater.go Adds the union eligibility helper and guards discriminant matching.
tsc/​internal/​checker/​jsx.go Adds an early exit for JSX contextual discrimination.
tsc/​internal/​checker/​checker.go Adds an early exit for object-literal contextual discrimination.

Review effort: Balanced

Comment thread tsc/internal/checker/checker.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Both contextual fast paths bypass existing caching, causing repeated union filtering and allocations for property and attribute queries.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment on lines +31400 to +31401
if !isDiscriminableUnion(contextualType) {
return c.removeNonDiscriminableConstituents(contextualType)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I measured this and couldn't find this to make any different for any project. Also, the result is node-independent so the per-node cache wouldnt fit this all too well. I could add a second type-based cache but I couldn't prove anyhow it would actually be worth it.

This branch has not been deployed

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

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

2 participants