Skip to content
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

Avoid subtype reduction when creating a union result in discriminateTypeByDiscriminableItems #54052

Conversation

Andarist
Copy link
Contributor

fixes #54048 (which is a recent regression from https://github.com/microsoft/TypeScript/pull/53709/files , cc @ahejlsberg )

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 28, 2023
@jakebailey
Copy link
Member

@typescript-bot test this
@typescript-bot test top100
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this faster
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the extended test suite on this PR at 1ef6b24. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the tarball bundle task on this PR at 1ef6b24. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

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/153488/artifacts?artifactName=tgz&fileId=58624324DE8EC1191EA9F435313E02981A83B8D982B7E34D31A749096E3A7DCE02&fileName=/typescript-5.1.0-insiders.20230428.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/pr-build@5.1.0-pr-54052-7".;

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/54052/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..54052

Metric main 54052 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 365,294k (± 0.01%) 365,345k (± 0.01%) +51k (+ 0.01%) 365,270k 365,399k p=0.045 n=6
Parse Time 3.55s (± 0.48%) 3.54s (± 0.46%) ~ 3.52s 3.56s p=0.684 n=6
Bind Time 1.17s (± 0.47%) 1.17s (± 0.35%) ~ 1.17s 1.18s p=0.282 n=6
Check Time 9.56s (± 0.33%) 9.59s (± 0.50%) ~ 9.51s 9.65s p=0.295 n=6
Emit Time 7.89s (± 0.41%) 7.96s (± 0.78%) +0.08s (+ 0.95%) 7.85s 8.03s p=0.045 n=6
Total Time 22.18s (± 0.22%) 22.27s (± 0.50%) ~ 22.06s 22.40s p=0.065 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,803k (± 0.90%) 192,728k (± 0.03%) ~ 192,639k 192,812k p=1.000 n=6
Parse Time 1.60s (± 0.51%) 1.60s (± 0.86%) ~ 1.57s 1.61s p=1.000 n=6
Bind Time 0.82s (± 0.63%) 0.82s (± 0.00%) ~ 0.82s 0.82s p=0.174 n=6
Check Time 10.22s (± 0.49%) 10.30s (± 0.63%) ~ 10.22s 10.38s p=0.065 n=6
Emit Time 3.00s (± 1.32%) 3.04s (± 1.64%) ~ 2.99s 3.12s p=0.147 n=6
Total Time 15.64s (± 0.30%) 15.76s (± 0.61%) +0.11s (+ 0.71%) 15.61s 15.88s p=0.045 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,853k (± 0.00%) 345,864k (± 0.01%) ~ 345,842k 345,897k p=0.422 n=6
Parse Time 2.72s (± 0.43%) 2.72s (± 0.23%) ~ 2.71s 2.73s p=1.000 n=6
Bind Time 1.09s (± 0.75%) 1.08s (± 0.38%) -0.01s (- 0.77%) 1.07s 1.08s p=0.038 n=6
Check Time 7.89s (± 0.57%) 7.90s (± 0.62%) ~ 7.83s 7.97s p=0.686 n=6
Emit Time 4.49s (± 0.81%) 4.50s (± 0.71%) ~ 4.46s 4.55s p=0.517 n=6
Total Time 16.18s (± 0.54%) 16.21s (± 0.43%) ~ 16.09s 16.28s p=0.575 n=6
TFS - node (v16.17.1, x64)
Memory used 300,100k (± 0.01%) 300,101k (± 0.01%) ~ 300,062k 300,136k p=1.000 n=6
Parse Time 2.15s (± 0.54%) 2.15s (± 0.54%) ~ 2.14s 2.17s p=1.000 n=6
Bind Time 1.23s (± 0.61%) 1.24s (± 0.33%) ~ 1.23s 1.24s p=0.100 n=6
Check Time 7.29s (± 0.27%) 7.30s (± 0.23%) ~ 7.27s 7.32s p=0.195 n=6
Emit Time 4.39s (± 0.81%) 4.38s (± 0.89%) ~ 4.33s 4.43s p=0.468 n=6
Total Time 15.06s (± 0.30%) 15.06s (± 0.42%) ~ 14.97s 15.14s p=1.000 n=6
material-ui - node (v16.17.1, x64)
Memory used 481,600k (± 0.00%) 481,692k (± 0.01%) +92k (+ 0.02%) 481,654k 481,753k p=0.005 n=6
Parse Time 3.22s (± 0.25%) 3.24s (± 0.30%) +0.02s (+ 0.47%) 3.23s 3.25s p=0.026 n=6
Bind Time 0.94s (± 0.80%) 0.93s (± 0.55%) ~ 0.93s 0.94s p=0.247 n=6
Check Time 17.90s (± 0.49%) 18.41s (± 1.24%) +0.51s (+ 2.83%) 18.17s 18.83s p=0.005 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.06s (± 0.39%) 22.58s (± 0.97%) +0.52s (+ 2.35%) 22.36s 22.98s p=0.005 n=6
xstate - node (v16.17.1, x64)
Memory used 560,694k (± 0.02%) 560,660k (± 0.02%) ~ 560,538k 560,879k p=0.471 n=6
Parse Time 3.97s (± 0.35%) 3.99s (± 0.47%) ~ 3.97s 4.02s p=0.243 n=6
Bind Time 1.76s (± 0.56%) 1.76s (± 0.47%) ~ 1.76s 1.78s p=0.273 n=6
Check Time 3.05s (± 0.13%) 3.06s (± 0.27%) +0.01s (+ 0.49%) 3.05s 3.07s p=0.010 n=6
Emit Time 0.09s (± 0.00%) 0.09s (± 0.00%) ~ 0.09s 0.09s p=1.000 n=6
Total Time 8.88s (± 0.22%) 8.90s (± 0.15%) +0.03s (+ 0.30%) 8.88s 8.92s p=0.042 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-148-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 54052 6
Baseline main 6

Developer Information:

Download Benchmark

@jakebailey
Copy link
Member

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 28, 2023

Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..54052

Metric main 54052 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 365,294k (± 0.01%) 365,333k (± 0.00%) +39k (+ 0.01%) 365,315k 365,350k p=0.016 n=6
Parse Time 3.55s (± 0.48%) 3.53s (± 0.76%) ~ 3.51s 3.58s p=0.145 n=6
Bind Time 1.17s (± 0.47%) 1.17s (± 0.44%) ~ 1.17s 1.18s p=0.640 n=6
Check Time 9.56s (± 0.33%) 9.59s (± 0.29%) ~ 9.57s 9.64s p=0.293 n=6
Emit Time 7.89s (± 0.41%) 7.96s (± 0.84%) ~ 7.87s 8.04s p=0.108 n=6
Total Time 22.18s (± 0.22%) 22.25s (± 0.24%) +0.07s (+ 0.33%) 22.16s 22.30s p=0.037 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,803k (± 0.90%) 192,698k (± 0.02%) ~ 192,645k 192,774k p=0.689 n=6
Parse Time 1.60s (± 0.51%) 1.59s (± 0.95%) ~ 1.57s 1.60s p=0.213 n=6
Bind Time 0.82s (± 0.63%) 0.82s (± 0.66%) ~ 0.82s 0.83s p=0.640 n=6
Check Time 10.22s (± 0.49%) 10.33s (± 0.58%) +0.11s (+ 1.06%) 10.25s 10.40s p=0.013 n=6
Emit Time 3.00s (± 1.32%) 3.02s (± 1.35%) ~ 2.98s 3.08s p=0.423 n=6
Total Time 15.64s (± 0.30%) 15.77s (± 0.58%) +0.12s (+ 0.78%) 15.62s 15.88s p=0.045 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,853k (± 0.00%) 345,856k (± 0.00%) ~ 345,844k 345,871k p=0.936 n=6
Parse Time 2.72s (± 0.43%) 2.73s (± 0.61%) ~ 2.71s 2.75s p=0.410 n=6
Bind Time 1.09s (± 0.75%) 1.08s (± 0.59%) ~ 1.07s 1.09s p=0.097 n=6
Check Time 7.89s (± 0.57%) 7.91s (± 0.37%) ~ 7.87s 7.94s p=0.373 n=6
Emit Time 4.49s (± 0.81%) 4.50s (± 0.54%) ~ 4.47s 4.53s p=1.000 n=6
Total Time 16.18s (± 0.54%) 16.22s (± 0.35%) ~ 16.15s 16.31s p=0.469 n=6
TFS - node (v16.17.1, x64)
Memory used 300,100k (± 0.01%) 300,113k (± 0.01%) ~ 300,089k 300,132k p=0.378 n=6
Parse Time 2.15s (± 0.54%) 2.16s (± 0.63%) ~ 2.14s 2.17s p=0.562 n=6
Bind Time 1.23s (± 0.61%) 1.24s (± 0.83%) ~ 1.22s 1.25s p=0.351 n=6
Check Time 7.29s (± 0.27%) 7.26s (± 0.54%) ~ 7.23s 7.32s p=0.259 n=6
Emit Time 4.39s (± 0.81%) 4.39s (± 1.01%) ~ 4.34s 4.44s p=0.936 n=6
Total Time 15.06s (± 0.30%) 15.05s (± 0.56%) ~ 14.96s 15.17s p=0.872 n=6
material-ui - node (v16.17.1, x64)
Memory used 481,600k (± 0.00%) 481,688k (± 0.02%) +88k (+ 0.02%) 481,629k 481,870k p=0.008 n=6
Parse Time 3.22s (± 0.25%) 3.24s (± 0.23%) +0.02s (+ 0.47%) 3.23s 3.25s p=0.015 n=6
Bind Time 0.94s (± 0.80%) 0.93s (± 0.81%) ~ 0.92s 0.94s p=0.195 n=6
Check Time 17.90s (± 0.49%) 18.15s (± 0.56%) +0.25s (+ 1.40%) 18.05s 18.29s p=0.005 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.06s (± 0.39%) 22.32s (± 0.46%) +0.26s (+ 1.19%) 22.22s 22.47s p=0.005 n=6
xstate - node (v16.17.1, x64)
Memory used 560,694k (± 0.02%) 560,739k (± 0.02%) ~ 560,598k 560,867k p=0.575 n=6
Parse Time 3.97s (± 0.35%) 3.97s (± 0.19%) ~ 3.96s 3.98s p=0.389 n=6
Bind Time 1.76s (± 0.56%) 1.76s (± 0.43%) ~ 1.75s 1.77s p=0.554 n=6
Check Time 3.05s (± 0.13%) 3.07s (± 0.39%) +0.02s (+ 0.60%) 3.05s 3.08s p=0.010 n=6
Emit Time 0.09s (± 0.00%) 0.09s (± 0.00%) ~ 0.09s 0.09s p=1.000 n=6
Total Time 8.88s (± 0.22%) 8.89s (± 0.20%) ~ 8.86s 8.91s p=0.258 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-148-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 54052 6
Baseline main 6

Developer Information:

Download Benchmark

@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 top-repos suite comparing main and refs/pull/54052/merge:

Everything looks good!

@sandersn sandersn added this to Not started in PR Backlog May 8, 2023
@sandersn sandersn moved this from Not started to Waiting on reviewers in PR Backlog May 24, 2023
@Andarist
Copy link
Contributor Author

this is for 5.1 regression, is there any chance to get some eyes on it before the stable release?

@DanielRosenwasser
Copy link
Member

@typescript-bot test this
@typescript-bot test top100
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this faster
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 1ef6b24. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 1ef6b24. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Heya @DanielRosenwasser, I've started to run the diff-based user code test suite on this PR at 1ef6b24. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 26, 2023

Hey @DanielRosenwasser, 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/155020/artifacts?artifactName=tgz&fileId=371BFA1AE731F4CD85C993428125E1F040D1F3F411588EDE7261519EA824D12602&fileName=/typescript-5.2.0-insiders.20230526.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/pr-build@5.2.0-pr-54052-23".;

@ahejlsberg
Copy link
Member

In general I think this is a good change, we really shouldn't be subtype reducing contextual types (elsewhere we specifically avoid it). Seems like there's a slight performance regression in material-ui, but may just be that we're giving back the slight gain we got in #53709. Plus, TBH I'm not sure what we're really measuring with that old snapshot at this point.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..54052

Metric main 54052 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 365,242k (± 0.01%) 365,247k (± 0.01%) ~ 365,187k 365,303k p=0.936 n=6
Parse Time 3.56s (± 0.49%) 3.57s (± 0.46%) ~ 3.54s 3.58s p=0.744 n=6
Bind Time 1.18s (± 1.15%) 1.19s (± 0.34%) ~ 1.19s 1.20s p=0.275 n=6
Check Time 9.55s (± 0.58%) 9.62s (± 0.80%) ~ 9.52s 9.71s p=0.108 n=6
Emit Time 7.88s (± 0.46%) 7.96s (± 1.01%) ~ 7.86s 8.07s p=0.064 n=6
Total Time 22.18s (± 0.41%) 22.34s (± 0.63%) ~ 22.15s 22.47s p=0.053 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,524k (± 0.71%) 192,919k (± 0.03%) ~ 192,836k 192,986k p=0.375 n=6
Parse Time 1.59s (± 1.52%) 1.60s (± 1.16%) ~ 1.57s 1.62s p=0.511 n=6
Bind Time 0.82s (± 0.50%) 0.83s (± 0.49%) +0.01s (+ 1.22%) 0.83s 0.84s p=0.008 n=6
Check Time 10.13s (± 0.70%) 10.22s (± 0.62%) +0.09s (+ 0.90%) 10.13s 10.30s p=0.045 n=6
Emit Time 3.00s (± 0.88%) 3.01s (± 0.66%) ~ 2.98s 3.03s p=0.406 n=6
Total Time 15.55s (± 0.53%) 15.67s (± 0.43%) +0.12s (+ 0.75%) 15.56s 15.75s p=0.030 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,891k (± 0.00%) 345,891k (± 0.00%) ~ 345,876k 345,916k p=0.935 n=6
Parse Time 2.73s (± 0.97%) 2.74s (± 0.43%) ~ 2.73s 2.76s p=0.370 n=6
Bind Time 1.08s (± 0.48%) 1.09s (± 0.37%) ~ 1.08s 1.09s p=0.112 n=6
Check Time 7.85s (± 0.23%) 7.87s (± 0.36%) ~ 7.84s 7.90s p=0.368 n=6
Emit Time 4.46s (± 0.93%) 4.49s (± 0.64%) ~ 4.46s 4.53s p=0.261 n=6
Total Time 16.12s (± 0.47%) 16.19s (± 0.30%) ~ 16.14s 16.25s p=0.170 n=6
TFS - node (v16.17.1, x64)
Memory used 299,951k (± 0.01%) 299,965k (± 0.01%) ~ 299,947k 299,994k p=0.423 n=6
Parse Time 2.16s (± 0.35%) 2.18s (± 0.84%) ~ 2.16s 2.21s p=0.063 n=6
Bind Time 1.24s (± 0.42%) 1.24s (± 0.61%) ~ 1.23s 1.25s p=0.241 n=6
Check Time 7.27s (± 0.20%) 7.28s (± 0.45%) ~ 7.24s 7.32s p=0.872 n=6
Emit Time 4.33s (± 0.72%) 4.34s (± 0.56%) ~ 4.30s 4.37s p=0.413 n=6
Total Time 15.00s (± 0.28%) 15.04s (± 0.23%) ~ 15.02s 15.11s p=0.089 n=6
material-ui - node (v16.17.1, x64)
Memory used 480,988k (± 0.00%) 481,071k (± 0.01%) +83k (+ 0.02%) 481,010k 481,179k p=0.013 n=6
Parse Time 3.25s (± 0.34%) 3.26s (± 0.32%) ~ 3.24s 3.27s p=0.281 n=6
Bind Time 0.94s (± 0.55%) 0.94s (± 0.43%) ~ 0.94s 0.95s p=0.114 n=6
Check Time 17.80s (± 0.30%) 18.35s (± 0.60%) +0.55s (+ 3.10%) 18.16s 18.45s p=0.005 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 21.98s (± 0.23%) 22.55s (± 0.48%) +0.57s (+ 2.58%) 22.37s 22.67s p=0.005 n=6
xstate - node (v16.17.1, x64)
Memory used 560,637k (± 0.01%) 560,571k (± 0.02%) ~ 560,488k 560,710k p=0.173 n=6
Parse Time 4.00s (± 0.44%) 4.01s (± 0.60%) ~ 3.99s 4.06s p=0.732 n=6
Bind Time 1.77s (± 0.29%) 1.76s (± 0.59%) ~ 1.75s 1.78s p=0.437 n=6
Check Time 3.06s (± 0.74%) 3.08s (± 0.70%) ~ 3.04s 3.09s p=0.249 n=6
Emit Time 0.09s (± 0.00%) 0.09s (± 0.00%) ~ 0.09s 0.09s p=1.000 n=6
Total Time 8.91s (± 0.38%) 8.94s (± 0.41%) ~ 8.91s 9.01s p=0.125 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-148-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 54052 6
Baseline main 6

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

@DanielRosenwasser Here are the results of running the user test suite comparing main and refs/pull/54052/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

@typescript-bot
Copy link
Collaborator

Hey @DanielRosenwasser, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@typescript-bot
Copy link
Collaborator

@DanielRosenwasser Here are the results of running the top-repos suite comparing main and refs/pull/54052/merge:

Everything looks good!

@Andarist
Copy link
Contributor Author

It seems that this PR got preliminary approval from @ahejlsberg . I wonder if it could land before 5.1 (and be cherry-picked into it) since its due date was supposed to be... yesterday 😅 (according to the 5.1 iteration plan)

@jakebailey jakebailey added this to the TypeScript 5.1.4 milestone Jun 6, 2023
@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jun 6, 2023
@DanielRosenwasser
Copy link
Member

It seems that this PR got preliminary approval from @ahejlsberg .

Sorry, can you link back to where? @weswigham can you take a look?

@Andarist
Copy link
Contributor Author

@DanielRosenwasser here:

In general I think this is a good change, we really shouldn't be subtype reducing contextual types (elsewhere we specifically avoid it)

I called it preliminary since it didnt come with a more formal ✅ :P

@DanielRosenwasser DanielRosenwasser merged commit a3773ec into microsoft:main Jun 27, 2023
17 checks passed
PR Backlog automation moved this from Waiting on reviewers to Done Jun 27, 2023
@Andarist Andarist deleted the avoid-subtype-reduction-on-discriminated-items branch June 27, 2023 19:04
@DanielRosenwasser
Copy link
Member

Just documenting this one as a consistent 0.3-0.4 second regression.

image

@jakebailey
Copy link
Member

Original PR which this PR fixes up shows an improvement, so it may be a wash maybe but not entirely :\

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Comparing a tuple input to a variadic tuple no longer narrows it
5 participants