Skip to content

in operator shouldn't narrow {} originating in unknown #50610

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.

Sign up for GitHub

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

Jump to bottom
Merged
ahejlsberg merged 2 commits into from
Sep 2, 2022
Merged

in operator shouldn't narrow {} originating in unknown #50610

ahejlsberg merged 2 commits into from
Sep 2, 2022

Conversation

@ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Sep 2, 2022

Fixes #50531.

This is a targeted fix for the exact issue in #50531. It doesn't address other known issues with the in operator.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Sep 2, 2022
@ahejlsberg
Copy link
Member Author

ahejlsberg commented Sep 2, 2022

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022 •
edited
Loading

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 517777d. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022

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

Here they are:

Comparison Report - main..50610

Metric main 50610 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 338,741k (± 0.01%) 338,732k (± 0.01%) -9k (- 0.00%) 338,679k 338,780k
Parse Time 2.08s (± 0.72%) 2.09s (± 1.27%) +0.01s (+ 0.67%) 2.05s 2.16s
Bind Time 0.80s (± 1.52%) 0.79s (± 0.51%) -0.01s (- 1.00%) 0.78s 0.80s
Check Time 5.84s (± 0.30%) 5.83s (± 0.48%) -0.01s (- 0.24%) 5.78s 5.90s
Emit Time 6.18s (± 0.96%) 6.17s (± 0.52%) -0.01s (- 0.16%) 6.10s 6.23s
Total Time 14.90s (± 0.48%) 14.88s (± 0.26%) -0.01s (- 0.09%) 14.79s 14.98s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,975k (± 0.38%) 192,983k (± 0.39%) +8k (+ 0.00%) 192,579k 195,996k
Parse Time 0.86s (± 1.15%) 0.85s (± 0.80%) -0.00s (- 0.47%) 0.84s 0.87s
Bind Time 0.48s (± 1.03%) 0.49s (± 0.98%) +0.00s (+ 0.41%) 0.48s 0.50s
Check Time 6.69s (± 0.60%) 6.73s (± 0.41%) +0.03s (+ 0.51%) 6.67s 6.80s
Emit Time 2.41s (± 0.65%) 2.41s (± 0.62%) +0.00s (+ 0.04%) 2.37s 2.43s
Total Time 10.44s (± 0.45%) 10.47s (± 0.28%) +0.03s (+ 0.29%) 10.38s 10.53s
Monaco - node (v14.15.1, x64)
Memory used 326,527k (± 0.01%) 326,530k (± 0.01%) +3k (+ 0.00%) 326,469k 326,562k
Parse Time 1.57s (± 0.68%) 1.57s (± 0.53%) -0.00s (- 0.19%) 1.55s 1.58s
Bind Time 0.72s (± 0.50%) 0.72s (± 0.47%) -0.00s (- 0.14%) 0.72s 0.73s
Check Time 5.72s (± 0.51%) 5.73s (± 0.31%) +0.01s (+ 0.23%) 5.70s 5.77s
Emit Time 3.33s (± 0.87%) 3.30s (± 0.45%) -0.04s (- 1.14%) 3.27s 3.33s
Total Time 11.35s (± 0.36%) 11.32s (± 0.30%) -0.03s (- 0.25%) 11.25s 11.40s
TFS - node (v14.15.1, x64)
Memory used 289,647k (± 0.01%) 289,649k (± 0.01%) +2k (+ 0.00%) 289,594k 289,681k
Parse Time 1.31s (± 0.81%) 1.30s (± 0.91%) -0.00s (- 0.38%) 1.28s 1.34s
Bind Time 0.79s (± 0.51%) 0.80s (± 1.27%) +0.00s (+ 0.51%) 0.78s 0.83s
Check Time 5.36s (± 0.43%) 5.34s (± 0.42%) -0.02s (- 0.35%) 5.30s 5.39s
Emit Time 3.60s (± 1.19%) 3.56s (± 1.05%) -0.03s (- 0.97%) 3.49s 3.68s
Total Time 11.06s (± 0.52%) 11.00s (± 0.48%) -0.06s (- 0.56%) 10.88s 11.16s
material-ui - node (v14.15.1, x64)
Memory used 436,456k (± 0.00%) 436,461k (± 0.01%) +5k (+ 0.00%) 436,416k 436,518k
Parse Time 1.86s (± 0.38%) 1.87s (± 0.56%) +0.01s (+ 0.48%) 1.85s 1.89s
Bind Time 0.58s (± 0.63%) 0.58s (± 0.51%) -0.00s (- 0.51%) 0.58s 0.59s
Check Time 12.86s (± 0.51%) 12.90s (± 0.80%) +0.04s (+ 0.32%) 12.66s 13.21s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.30s (± 0.41%) 15.35s (± 0.64%) +0.05s (+ 0.33%) 15.11s 15.64s
xstate - node (v14.15.1, x64)
Memory used 546,724k (± 0.00%) 546,737k (± 0.00%) +13k (+ 0.00%) 546,714k 546,782k
Parse Time 2.60s (± 0.43%) 2.60s (± 0.52%) -0.00s (- 0.12%) 2.58s 2.64s
Bind Time 0.97s (± 0.77%) 0.98s (± 0.85%) +0.01s (+ 0.93%) 0.96s 1.00s
Check Time 1.53s (± 0.65%) 1.53s (± 0.68%) -0.00s (- 0.13%) 1.51s 1.55s
Emit Time 0.07s (± 4.13%) 0.07s (± 3.14%) -0.00s (- 1.39%) 0.07s 0.08s
Total Time 5.18s (± 0.31%) 5.18s (± 0.41%) 0.00s ( 0.00%) 5.13s 5.23s

System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 50610 10
Baseline main 10

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022

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

Everything looks good!

@andrewbranch andrewbranch added this to the TypeScript 4.8.3 milestone Sep 2, 2022
@andrewbranch
Copy link
Member

andrewbranch commented Sep 2, 2022

@typescript-bot cherry-pick this to release-4.8

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022 •
edited
Loading

Heya @andrewbranch, I've started to run the task to cherry-pick this into release-4.8 on this PR at 517777d. You can monitor the build here.

@typescript-bot typescript-bot mentioned this pull request Sep 2, 2022
Merged
@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 2, 2022

Hey @andrewbranch, I've opened #50612 for you.

@ahejlsberg ahejlsberg merged commit 854d448 into main Sep 2, 2022
@ahejlsberg ahejlsberg deleted the fix50531 branch September 2, 2022 16:30
@fatcerberus
Copy link

fatcerberus commented Sep 2, 2022

What was the cause of #50531? It’s weird that unknown -> {} got narrowed by in but a directly-typed {} didn’t.

@andrewbranch
Copy link
Member

andrewbranch commented Sep 2, 2022

It’s that weird declaredType !== type condition—we discussed rewriting that whole set of conditions for 4.9 such that in only filters object types out of unions by property when there’s more than one object-like type in the declared type. There was also some discussion about whether to do anything if the property tested by in doesn’t exist in any of the object-like types in the union. (@ahejlsberg can probably better explain where we landed.)

@sync-by-unito sync-by-unito bot mentioned this pull request Sep 13, 2022
Closed
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@andrewbranch andrewbranch andrewbranch approved these changes

Assignees

@ahejlsberg ahejlsberg

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Milestone

TypeScript 4.8.3

Development

Successfully merging this pull request may close these issues.

Different behavior for {} and unknown narrowed to {}

5 participants

@ahejlsberg @typescript-bot @andrewbranch @fatcerberus