Skip to content

Empty object not subtype of object with index signature - #50013

Closed
Anders Hejlsberg (ahejlsberg) wants to merge 3 commits into
mainfrom
emptyObjectAndIndexSignatures
Closed

Empty object not subtype of object with index signature#50013
Anders Hejlsberg (ahejlsberg) wants to merge 3 commits into
mainfrom
emptyObjectAndIndexSignatures

Conversation

@ahejlsberg

@ahejlsbergAnders Hejlsberg (ahejlsberg) commented Jul 22, 2022

Copy link
Copy Markdown
Member

This PR tightens the subtype relation such that a non-fresh {} is not considered a subtype of { [x: string]: xxx }. Currently, such types are subtypes in both directions, which causes issues in narrowing.

This fixes the issue reported here.

@typescript-botTypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jul 22, 2022
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Jul 22, 2022

Copy link
Copy Markdown
MemberAuthor

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test this inline
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) perf test faster

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 22, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at c7b2942. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 22, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the parallelized Definitely Typed test suite on this PR at c7b2942. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 22, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the abridged perf test suite on this PR at c7b2942. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 22, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

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

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..50013

Metricmain50013DeltaBestWorst
Angular - node (v14.15.1, x64)
Memory used335,975k (± 0.00%)335,968k (± 0.01%)-7k (- 0.00%)335,931k336,017k
Parse Time2.07s (± 0.81%)2.06s (± 0.65%)-0.01s (- 0.58%)2.04s2.11s
Bind Time0.89s (± 0.75%)0.89s (± 0.53%)0.00s ( 0.00%)0.88s0.90s
Check Time5.82s (± 0.52%)5.79s (± 0.28%)-0.02s (- 0.38%)5.76s5.83s
Emit Time6.39s (± 0.92%)6.36s (± 0.40%)-0.03s (- 0.53%)6.28s6.40s
Total Time15.17s (± 0.49%)15.10s (± 0.29%)-0.07s (- 0.45%)14.99s15.18s
Compiler-Unions - node (v14.15.1, x64)
Memory used193,150k (± 0.01%)193,461k (± 0.38%)+310k (+ 0.16%)193,049k196,403k
Parse Time0.85s (± 0.40%)0.85s (± 0.99%)+0.00s (+ 0.24%)0.83s0.87s
Bind Time0.57s (± 0.82%)0.57s (± 0.59%)+0.00s (+ 0.53%)0.57s0.58s
Check Time6.74s (± 0.51%)6.79s (± 0.90%)+0.05s (+ 0.76%)6.70s6.98s
Emit Time2.49s (± 1.01%)2.50s (± 0.90%)+0.00s (+ 0.20%)2.45s2.56s
Total Time10.65s (± 0.39%)10.71s (± 0.53%)+0.06s (+ 0.59%)10.59s10.85s
Monaco - node (v14.15.1, x64)
Memory used325,632k (± 0.01%)325,656k (± 0.01%)+24k (+ 0.01%)325,611k325,689k
Parse Time1.58s (± 0.53%)1.58s (± 0.71%)-0.00s (- 0.13%)1.56s1.61s
Bind Time0.78s (± 0.43%)0.78s (± 0.74%)0.00s ( 0.00%)0.77s0.79s
Check Time5.69s (± 0.70%)5.70s (± 0.39%)+0.01s (+ 0.09%)5.67s5.77s
Emit Time3.37s (± 0.85%)3.37s (± 0.76%)+0.00s (+ 0.03%)3.33s3.42s
Total Time11.43s (± 0.50%)11.43s (± 0.40%)+0.00s (+ 0.03%)11.35s11.57s
TFS - node (v14.15.1, x64)
Memory used288,810k (± 0.01%)288,815k (± 0.01%)+5k (+ 0.00%)288,754k288,858k
Parse Time1.33s (± 1.56%)1.33s (± 1.27%)-0.00s (- 0.08%)1.29s1.36s
Bind Time0.78s (± 5.11%)0.76s (± 4.39%)-0.02s (- 2.18%)0.73s0.85s
Check Time5.36s (± 0.43%)5.36s (± 0.49%)+0.00s (+ 0.04%)5.30s5.43s
Emit Time3.63s (± 1.98%)3.55s (± 1.78%)-0.08s (- 2.20%)3.46s3.70s
Total Time11.10s (± 0.90%)11.01s (± 0.67%)-0.10s (- 0.86%)10.87s11.23s
material-ui - node (v14.15.1, x64)
Memory used446,719k (± 0.01%)446,522k (± 0.08%)-197k (- 0.04%)445,576k446,790k
Parse Time1.87s (± 0.35%)1.87s (± 0.36%)+0.00s (+ 0.05%)1.86s1.89s
Bind Time0.72s (± 0.77%)0.73s (± 1.21%)+0.01s (+ 1.25%)0.71s0.75s
Check Time13.14s (± 0.55%)13.27s (± 0.74%)+0.13s (+ 0.95%)13.01s13.46s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time15.74s (± 0.47%)15.87s (± 0.63%)+0.13s (+ 0.83%)15.59s16.05s
xstate - node (v14.15.1, x64)
Memory used541,312k (± 0.00%)541,318k (± 0.00%)+6k (+ 0.00%)541,281k541,377k
Parse Time2.61s (± 0.34%)2.61s (± 0.42%)-0.01s (- 0.27%)2.58s2.63s
Bind Time1.15s (± 1.42%)1.15s (± 1.09%)-0.01s (- 0.69%)1.12s1.18s
Check Time1.56s (± 0.65%)1.55s (± 0.57%)-0.00s (- 0.13%)1.54s1.57s
Emit Time0.07s (± 3.14%)0.07s (± 4.66%)+0.00s (+ 2.82%)0.07s0.08s
Total Time5.39s (± 0.26%)5.37s (± 0.41%)-0.02s (- 0.39%)5.33s5.43s
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)
BenchmarkNameIterations
Current5001310
Baselinemain10

Developer Information:

Download Benchmark

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg)
Great news! no new errors were found between main..refs/pull/50013/merge

Andarist
Mateusz Burzyński (Andarist) reviewed Jul 23, 2022
View reviewed changes
Comment threadtests/baselines/reference/useObjectValuesAndEntries1.types
Comment on lines +88 to +89
>values3 : any[]
>Object.values({}) : any[]

@AndaristMateusz Burzyński (Andarist)Jul 23, 2022

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.

q: is this a desired change?

@ahejlsbergAnders Hejlsberg (ahejlsberg) mentioned this pull request Jul 25, 2022
Merged
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Jul 26, 2022

Copy link
Copy Markdown
MemberAuthor

Closing in favor of #50044.

@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasserDaniel Rosenwasser (DanielRosenwasser)Awaiting requested review from DanielRosenwasser
@sandersnNathan Shively-Sanders (sandersn)Awaiting requested review from sandersn
2 more reviewers
@AndaristMateusz Burzyński (Andarist)Andarist left review comments
@ProKashifKashif Ahmed (ProKashif)ProKashif approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ahejlsbergAnders Hejlsberg (ahejlsberg)

Labels

Author: TeamFor Uncommitted BugPR for untriaged, rejected, closed or missing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ahejlsberg@typescript-bot@Andarist@ProKashif