Skip to content

Conversation

Andarist
Copy link
Contributor

fixes #46680
fixes #53947

@Copilot Copilot AI review requested due to automatic review settings August 28, 2025 17:30
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Aug 28, 2025
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 28, 2025
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #46680. If you can get it accepted, this PR will have a better chance of being reviewed.

1 similar comment
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #46680. If you can get it accepted, this PR will have a better chance of being reviewed.

Copy link

@Copilot 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

This PR implements control flow analysis for destructured rest variables, addressing two specific TypeScript issues (#46680 and #53947). The changes enable the TypeScript compiler to properly narrow types in destructuring assignments that include rest elements, improving type inference when discriminated unions are involved.

Key Changes

  • Enhanced control flow analysis to handle destructured rest variables by tracking narrowed types through rest patterns
  • Modified the constraint substitution logic to be more selective, improving type narrowing for binding elements
  • Extended support for both object and array destructuring with rest elements

Reviewed Changes

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

Show a summary per file
File Description
tests/cases/conformance/controlFlow/dependentDestructuredRestVariables1.tsx New test cases demonstrating control flow analysis for destructured rest variables in various scenarios
tests/baselines/reference/dependentDestructuredRestVariables1.symbols Generated symbol baseline for the new test cases
tests/baselines/reference/arrayDestructuringInSwitch1.types Updated type baseline showing improved type inference for array destructuring
tests/baselines/reference/arrayDestructuringInSwitch1.symbols Updated symbol baseline with refined symbol references
src/compiler/checker.ts Core implementation changes to enable control flow analysis for rest variables
Comments suppressed due to low confidence (1)

src/compiler/checker.ts:1

  • [nitpick] The long conditional expression on line 30851 reduces readability. Consider extracting the check mode determination into a variable: const checkMode = shouldSubstituteConstraints(type, location) ? CheckMode.Normal : CheckMode.SkipConstraintsSubstitution;
import {

const parent = declaration.parent.parent;
const rootDeclaration = getRootDeclaration(parent);
if (rootDeclaration.kind === SyntaxKind.VariableDeclaration && getCombinedNodeFlagsCached(rootDeclaration) & NodeFlags.Constant || rootDeclaration.kind === SyntaxKind.Parameter) {
const links = getNodeLinks(parent);
if (!(links.flags & NodeCheckFlags.InCheckIdentifier)) {
links.flags |= NodeCheckFlags.InCheckIdentifier;
const parentType = getTypeForBindingElementParent(parent, CheckMode.Normal);
const parentTypeConstraint = parentType && mapType(parentType, getBaseConstraintOrType);
const parentType = getTypeForBindingElementParent(parent, shouldSubstituteConstraints(type, location) ? CheckMode.Normal : CheckMode.SkipConstraintsSubstitution);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When obtaining the parent type we don't want to get its constraints substituted based on its contextual type or it being in a constraint position or something. This would lead to returning its constraint and then using that for getBindingElementTypeFromParentType even when the current location isn't at the constraint position.

@Andarist
Copy link
Contributor Author

Huh, I'll have to investigate what's happening with the self-check

@Andarist Andarist marked this pull request as draft August 28, 2025 17:42
@controversial
Copy link

wowwww ❤️

@Andarist Andarist marked this pull request as ready for review August 29, 2025 10:16
Comment on lines +3 to +7
=== Performance Stats ===
Assignability cache: 2,500
Type Count: 10,000
Instantiation count: 100,000
Symbol count: 50,000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fwiw, those numbers are the same on main

@jakebailey
Copy link
Member

@typescript-bot test it
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 3, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 3, 2025

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/165974/artifacts?artifactName=tgz&fileId=52DAAF77649A969D6AD6E977625BE4630CCD2B7CCA26470AB84D4A471C7E021B02&fileName=/typescript-6.0.0-insiders.20250903.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/62350/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Git clone failed"

Otherwise...

Everything looks good!

@typescript-bot
Copy link
Collaborator

@jakebailey
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 34 34 ~ ~ ~ p=1.000 n=6
Symbols 62,370 62,370 ~ ~ ~ p=1.000 n=6
Types 50,386 50,386 ~ ~ ~ p=1.000 n=6
Memory used 194,123k (± 0.96%) 193,567k (± 0.75%) ~ 192,906k 196,506k p=0.810 n=6
Parse Time 1.30s (± 1.02%) 1.31s (± 1.12%) ~ 1.29s 1.32s p=0.181 n=6
Bind Time 0.73s 0.73s ~ ~ ~ p=1.000 n=6
Check Time 9.74s (± 0.34%) 9.74s (± 0.36%) ~ 9.68s 9.77s p=0.809 n=6
Emit Time 2.74s (± 0.71%) 2.74s (± 0.68%) ~ 2.71s 2.76s p=0.935 n=6
Total Time 14.51s (± 0.18%) 14.52s (± 0.36%) ~ 14.43s 14.58s p=0.373 n=6
angular-1 - node (v18.15.0, x64)
Errors 1 1 ~ ~ ~ p=1.000 n=6
Symbols 948,914 948,914 ~ ~ ~ p=1.000 n=6
Types 410,884 410,884 ~ ~ ~ p=1.000 n=6
Memory used 1,246,625k (± 0.00%) 1,246,615k (± 0.00%) ~ 1,246,567k 1,246,685k p=0.689 n=6
Parse Time 6.48s (± 0.49%) 6.55s (± 0.93%) +0.07s (+ 1.11%) 6.49s 6.63s p=0.020 n=6
Bind Time 1.87s (± 0.28%) 1.88s (± 0.52%) ~ 1.87s 1.90s p=0.091 n=6
Check Time 32.19s (± 0.44%) 32.23s (± 0.24%) ~ 32.09s 32.32s p=0.575 n=6
Emit Time 14.82s (± 0.27%) 14.83s (± 0.41%) ~ 14.76s 14.93s p=0.936 n=6
Total Time 55.36s (± 0.29%) 55.49s (± 0.23%) ~ 55.27s 55.62s p=0.128 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,545,910 2,546,549 +639 (+ 0.03%) ~ ~ p=0.001 n=6
Types 902,549 902,700 +151 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 2,830,793k (± 0.01%) 2,831,059k (± 0.01%) +266k (+ 0.01%) 2,830,864k 2,831,232k p=0.031 n=6
Parse Time 8.76s (± 0.31%) 8.77s (± 0.22%) ~ 8.74s 8.80s p=0.871 n=6
Bind Time 2.23s (± 0.57%) 2.23s (± 0.62%) ~ 2.21s 2.25s p=0.510 n=6
Check Time 86.65s (± 0.93%) 86.08s (± 0.44%) ~ 85.62s 86.53s p=0.199 n=6
Emit Time 0.62s (±125.83%) 0.94s (±105.15%) ~ 0.30s 2.25s p=0.591 n=6
Total Time 98.25s (± 0.94%) 98.02s (± 1.28%) ~ 96.96s 99.73s p=0.471 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,236,922 1,236,897 -25 (- 0.00%) ~ ~ p=0.001 n=6
Types 259,638 259,637 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 2,386,903k (± 0.01%) 2,386,832k (± 0.02%) ~ 2,386,346k 2,387,379k p=0.810 n=6
Parse Time 5.18s (± 1.31%) 5.26s (± 0.70%) ~ 5.21s 5.31s p=0.053 n=6
Bind Time 1.81s (± 0.70%) 1.81s (± 0.57%) ~ 1.80s 1.83s p=0.560 n=6
Check Time 35.04s (± 0.26%) 35.02s (± 0.28%) ~ 34.88s 35.16s p=0.810 n=6
Emit Time 3.05s (± 3.20%) 2.99s (± 0.87%) ~ 2.95s 3.03s p=0.106 n=6
Total Time 45.09s (± 0.41%) 45.10s (± 0.17%) ~ 44.97s 45.20s p=0.471 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,236,922 1,236,897 -25 (- 0.00%) ~ ~ p=0.001 n=6
Types 259,638 259,637 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 3,180,878k (± 0.03%) 3,060,136k (± 9.66%) ~ 2,456,199k 3,181,810k p=0.689 n=6
Parse Time 6.95s (± 0.90%) 6.90s (± 1.27%) ~ 6.72s 6.95s p=0.689 n=6
Bind Time 2.17s (± 0.89%) 2.17s (± 1.41%) ~ 2.12s 2.20s p=1.000 n=6
Check Time 42.57s (± 0.21%) 42.53s (± 0.22%) ~ 42.42s 42.64s p=0.689 n=6
Emit Time 3.54s (± 2.70%) 3.53s (± 3.48%) ~ 3.39s 3.71s p=0.688 n=6
Total Time 55.23s (± 0.20%) 55.13s (± 0.21%) ~ 54.98s 55.26s p=0.230 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 262,416 262,413 -3 (- 0.00%) ~ ~ p=0.001 n=6
Types 103,917 103,916 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 441,660k (± 0.02%) 441,610k (± 0.02%) ~ 441,527k 441,718k p=0.378 n=6
Parse Time 4.40s (± 0.69%) 4.40s (± 0.74%) ~ 4.34s 4.43s p=1.000 n=6
Bind Time 1.64s (± 1.00%) 1.63s (± 1.62%) ~ 1.60s 1.67s p=0.461 n=6
Check Time 23.41s (± 0.38%) 23.34s (± 0.52%) ~ 23.19s 23.51s p=0.297 n=6
Emit Time 1.90s (± 0.81%) 1.90s (± 1.32%) ~ 1.85s 1.92s p=1.000 n=6
Total Time 31.34s (± 0.32%) 31.26s (± 0.42%) ~ 31.12s 31.48s p=0.173 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 72 72 ~ ~ ~ p=1.000 n=6
Symbols 225,367 225,367 ~ ~ ~ p=1.000 n=6
Types 94,290 94,290 ~ ~ ~ p=1.000 n=6
Memory used 371,166k (± 0.01%) 371,266k (± 0.07%) ~ 371,122k 371,768k p=0.748 n=6
Parse Time 3.58s (± 1.41%) 3.59s (± 1.01%) ~ 3.54s 3.63s p=0.747 n=6
Bind Time 1.95s (± 0.90%) 1.96s (± 1.15%) ~ 1.94s 1.99s p=0.371 n=6
Check Time 20.38s (± 0.15%) 20.41s (± 0.45%) ~ 20.29s 20.55s p=0.630 n=6
Emit Time 0.00s (±244.70%) 0.00s ~ ~ ~ p=0.405 n=6
Total Time 25.91s (± 0.19%) 25.95s (± 0.43%) ~ 25.81s 26.15s p=0.687 n=6
vscode - node (v18.15.0, x64)
Errors 1 1 ~ ~ ~ p=1.000 n=6
Symbols 3,881,783 3,882,434 +651 (+ 0.02%) ~ ~ p=0.001 n=6
Types 1,223,225 1,223,353 +128 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 3,707,541k (± 0.01%) 3,707,900k (± 0.01%) +360k (+ 0.01%) 3,707,507k 3,708,119k p=0.045 n=6
Parse Time 18.86s (± 0.41%) 18.84s (± 0.43%) ~ 18.74s 18.96s p=0.872 n=6
Bind Time 6.10s (± 2.14%) 6.11s (± 1.86%) ~ 6.02s 6.33s p=0.872 n=6
Check Time 124.10s (± 1.49%) 124.13s (± 1.23%) ~ 122.33s 125.70s p=0.810 n=6
Emit Time 50.23s (±17.04%) 43.81s (±16.94%) ~ 37.36s 56.86s p=0.230 n=6
Total Time 199.29s (± 4.75%) 192.88s (± 4.55%) ~ 184.77s 207.50s p=0.298 n=6
webpack - node (v18.15.0, x64)
Errors 37 37 ~ ~ ~ p=1.000 n=6
Symbols 361,681 361,684 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 159,185 159,191 +6 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 518,962k (± 0.06%) 519,058k (± 0.03%) ~ 518,884k 519,330k p=0.471 n=6
Parse Time 4.62s (± 0.73%) 4.65s (± 0.66%) ~ 4.61s 4.69s p=0.195 n=6
Bind Time 1.94s (± 1.15%) 1.94s (± 2.30%) ~ 1.89s 2.00s p=0.809 n=6
Check Time 22.13s (± 1.26%) 22.19s (± 1.47%) ~ 21.92s 22.69s p=0.423 n=6
Emit Time 0.00s (±244.70%) 0.00s ~ ~ ~ p=0.405 n=6
Total Time 28.69s (± 0.93%) 28.77s (± 1.09%) ~ 28.51s 29.18s p=0.423 n=6
xstate-main - node (v18.15.0, x64)
Errors 30 30 ~ ~ ~ p=1.000 n=6
Symbols 668,461 668,477 +16 (+ 0.00%) ~ ~ p=0.001 n=6
Types 200,558 200,612 +54 (+ 0.03%) ~ ~ p=0.001 n=6
Memory used 573,984k (± 0.03%) 573,857k (± 0.02%) ~ 573,708k 574,026k p=0.173 n=6
Parse Time 4.30s (± 0.69%) 4.28s (± 1.10%) ~ 4.23s 4.35s p=0.520 n=6
Bind Time 1.32s (± 1.00%) 1.33s (± 1.02%) ~ 1.32s 1.35s p=0.209 n=6
Check Time 20.18s (± 1.43%) 20.48s (± 2.06%) ~ 19.94s 20.91s p=0.688 n=6
Emit Time 0.00s 0.00s (±244.70%) ~ 0.00s 0.01s p=0.405 n=6
Total Time 25.80s (± 1.02%) 26.10s (± 1.78%) ~ 25.50s 26.60s p=0.520 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/62350/merge:

Everything looks good!

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.

Control flow analysis for destructured rest element of discriminated union Keep context after using spread
4 participants