Skip to content

Remove undefined from optional properties when inferring to index signatures - #43086

Merged
Anders Hejlsberg (ahejlsberg) merged 2 commits into
masterfrom
fix43045
Mar 6, 2021
Merged

Remove undefined from optional properties when inferring to index signatures#43086
Anders Hejlsberg (ahejlsberg) merged 2 commits into
masterfrom
fix43045

Conversation

@ahejlsberg

@ahejlsbergAnders Hejlsberg (ahejlsberg) commented Mar 5, 2021

Copy link
Copy Markdown
Member

Fixes#43045.

@typescript-botTypeScript Bot (typescript-bot) added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Mar 5, 2021
DanielRosenwasser
Daniel Rosenwasser (DanielRosenwasser) reviewed Mar 5, 2021
View reviewed changes
Comment threadtests/baselines/reference/inferenceOptionalPropertiesToIndexSignatures.js
let a1 = foo(x1); // string | number
let a2 = foo(x2); // string | number | undefined
let a3 = foo(x3); // string | number
let a4 = foo(x4); // string | number

@DanielRosenwasserDaniel Rosenwasser (DanielRosenwasser)Mar 5, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm - seems like this is the only questionable one.

@ahejlsbergAnders Hejlsberg (ahejlsberg)Mar 5, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Effectively, because we don't have a separate missing type, undefined represents the missing state in optional properties, so we really should remove undefined when we're inferring to an index signature as index signatures represent the type of those properties that are present.

@RyanCavanaughRyan Cavanaugh (RyanCavanaugh)Mar 5, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feels like things are getting a bit untenable without missing; any plausible implementation of foo with a legal x4 could return undefined. We should revisit it.

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Mar 5, 2021

Copy link
Copy Markdown
Member

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

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized Definitely Typed test suite on this PR at 85815b0. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the extended test suite on this PR at 85815b0. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized community code test suite on this PR at 85815b0. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the perf test suite on this PR at 85815b0. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the abridged perf test suite on this PR at 85815b0. You can monitor the build here.

Update: The results are in!

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Mar 5, 2021•
edited
Loading

Copy link
Copy Markdown
Member

Check with Andrew Branch (@andrewbranch) too since he made the corresponding change.

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser)
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..43086

Metricmaster43086DeltaBestWorst
Angular - node (v10.16.3, x64)
Memory used348,535k (± 0.03%)348,503k (± 0.02%)-32k (- 0.01%)348,394k348,634k
Parse Time1.93s (± 0.80%)1.91s (± 0.27%)-0.02s (- 1.09%)1.90s1.92s
Bind Time0.83s (± 0.72%)0.83s (± 0.84%)-0.00s (- 0.48%)0.82s0.85s
Check Time5.06s (± 0.50%)5.02s (± 0.42%)-0.04s (- 0.89%)4.96s5.06s
Emit Time5.97s (± 0.85%)5.91s (± 0.59%)-0.06s (- 1.09%)5.85s6.02s
Total Time13.80s (± 0.50%)13.66s (± 0.36%)-0.13s (- 0.97%)13.58s13.80s
Compiler-Unions - node (v10.16.3, x64)
Memory used204,763k (± 0.03%)204,674k (± 0.12%)-89k (- 0.04%)203,762k204,950k
Parse Time0.78s (± 0.79%)0.78s (± 0.86%)-0.00s (- 0.51%)0.76s0.79s
Bind Time0.52s (± 1.39%)0.52s (± 0.90%)-0.00s (- 0.38%)0.51s0.53s
Check Time7.46s (± 0.49%)7.43s (± 0.74%)-0.03s (- 0.39%)7.32s7.58s
Emit Time2.59s (± 0.77%)2.58s (± 0.47%)-0.01s (- 0.23%)2.55s2.61s
Total Time11.35s (± 0.43%)11.31s (± 0.50%)-0.04s (- 0.38%)11.21s11.47s
Monaco - node (v10.16.3, x64)
Memory used356,625k (± 0.02%)356,633k (± 0.02%)+8k (+ 0.00%)356,434k356,824k
Parse Time1.57s (± 0.89%)1.56s (± 0.33%)-0.01s (- 0.57%)1.55s1.57s
Bind Time0.74s (± 1.05%)0.73s (± 0.93%)-0.01s (- 0.94%)0.72s0.75s
Check Time5.20s (± 0.56%)5.19s (± 0.58%)-0.01s (- 0.12%)5.10s5.26s
Emit Time3.14s (± 0.93%)3.12s (± 0.79%)-0.02s (- 0.64%)3.08s3.20s
Total Time10.65s (± 0.57%)10.61s (± 0.36%)-0.04s (- 0.40%)10.50s10.67s
TFS - node (v10.16.3, x64)
Memory used308,931k (± 0.02%)308,989k (± 0.03%)+58k (+ 0.02%)308,796k309,303k
Parse Time1.22s (± 0.82%)1.21s (± 0.30%)-0.01s (- 0.90%)1.20s1.21s
Bind Time0.70s (± 0.98%)0.69s (± 0.98%)-0.00s (- 0.29%)0.68s0.71s
Check Time4.69s (± 0.48%)4.69s (± 0.76%)+0.00s (+ 0.04%)4.62s4.76s
Emit Time3.24s (± 0.65%)3.23s (± 1.45%)-0.01s (- 0.25%)3.13s3.30s
Total Time9.85s (± 0.49%)9.82s (± 0.76%)-0.02s (- 0.23%)9.69s9.97s
material-ui - node (v10.16.3, x64)
Memory used501,518k (± 0.01%)501,585k (± 0.02%)+67k (+ 0.01%)501,400k501,778k
Parse Time2.00s (± 0.56%)1.99s (± 0.86%)-0.01s (- 0.35%)1.96s2.03s
Bind Time0.65s (± 1.30%)0.65s (± 0.95%)+0.00s (+ 0.00%)0.64s0.67s
Check Time14.28s (± 0.77%)14.23s (± 0.80%)-0.05s (- 0.33%)14.01s14.55s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time16.92s (± 0.67%)16.87s (± 0.74%)-0.05s (- 0.32%)16.62s17.23s
Angular - node (v12.1.0, x64)
Memory used326,265k (± 0.03%)326,149k (± 0.02%)-116k (- 0.04%)325,972k326,303k
Parse Time1.92s (± 0.99%)1.90s (± 0.46%)-0.02s (- 0.83%)1.89s1.93s
Bind Time0.81s (± 0.68%)0.81s (± 1.19%)-0.00s (- 0.37%)0.79s0.84s
Check Time4.92s (± 0.52%)4.93s (± 0.69%)+0.00s (+ 0.08%)4.87s5.01s
Emit Time6.03s (± 0.57%)6.00s (± 0.76%)-0.04s (- 0.65%)5.94s6.15s
Total Time13.69s (± 0.41%)13.63s (± 0.57%)-0.06s (- 0.40%)13.53s13.86s
Compiler-Unions - node (v12.1.0, x64)
Memory used191,416k (± 0.03%)191,370k (± 0.02%)-46k (- 0.02%)191,274k191,436k
Parse Time0.77s (± 0.62%)0.77s (± 0.84%)-0.01s (- 0.65%)0.76s0.78s
Bind Time0.53s (± 0.89%)0.52s (± 0.57%)-0.01s (- 2.26%)0.51s0.52s
Check Time7.00s (± 0.52%)6.87s (± 0.59%)-0.13s (- 1.91%)6.80s6.98s
Emit Time2.57s (± 1.05%)2.55s (± 0.46%)-0.02s (- 0.62%)2.51s2.57s
Total Time10.87s (± 0.37%)10.70s (± 0.46%)-0.17s (- 1.52%)10.60s10.82s
Monaco - node (v12.1.0, x64)
Memory used339,033k (± 0.02%)339,034k (± 0.04%)+1k (+ 0.00%)338,846k339,501k
Parse Time1.55s (± 0.79%)1.52s (± 0.66%)-0.03s (- 1.74%)1.50s1.54s
Bind Time0.72s (± 0.31%)0.72s (± 1.01%)-0.00s (- 0.42%)0.70s0.73s
Check Time5.05s (± 0.39%)5.00s (± 0.52%)-0.05s (- 1.07%)4.95s5.07s
Emit Time3.16s (± 0.71%)3.09s (± 0.68%)-0.07s (- 2.28%)3.04s3.12s
Total Time10.48s (± 0.28%)10.32s (± 0.43%)-0.16s (- 1.50%)10.25s10.43s
TFS - node (v12.1.0, x64)
Memory used293,104k (± 0.02%)293,153k (± 0.02%)+48k (+ 0.02%)293,033k293,350k
Parse Time1.23s (± 0.99%)1.22s (± 0.68%)-0.01s (- 0.82%)1.19s1.23s
Bind Time0.68s (± 0.73%)0.68s (± 0.65%)-0.00s (- 0.44%)0.67s0.69s
Check Time4.61s (± 0.71%)4.57s (± 0.40%)-0.04s (- 0.89%)4.53s4.62s
Emit Time3.18s (± 0.71%)3.11s (± 0.62%)-0.07s (- 2.08%)3.06s3.15s
Total Time9.70s (± 0.58%)9.58s (± 0.29%)-0.12s (- 1.20%)9.53s9.66s
material-ui - node (v12.1.0, x64)
Memory used479,454k (± 0.02%)479,495k (± 0.04%)+41k (+ 0.01%)479,015k479,783k
Parse Time2.00s (± 0.41%)2.00s (± 0.62%)-0.00s (- 0.10%)1.98s2.03s
Bind Time0.64s (± 0.76%)0.64s (± 0.52%)-0.00s (- 0.62%)0.63s0.65s
Check Time12.89s (± 0.68%)12.73s (± 0.85%)-0.16s (- 1.23%)12.57s13.10s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time15.54s (± 0.60%)15.37s (± 0.68%)-0.17s (- 1.09%)15.20s15.71s
Angular - node (v14.15.1, x64)
Memory used324,919k (± 0.01%)324,919k (± 0.01%)+0k (+ 0.00%)324,853k324,999k
Parse Time1.92s (± 0.91%)1.90s (± 0.55%)-0.02s (- 0.99%)1.88s1.93s
Bind Time0.85s (± 0.52%)0.84s (± 0.59%)-0.01s (- 0.82%)0.84s0.86s
Check Time4.91s (± 0.43%)4.91s (± 0.36%)-0.00s (- 0.04%)4.89s4.98s
Emit Time6.30s (± 0.42%)6.26s (± 0.33%)-0.04s (- 0.67%)6.22s6.32s
Total Time13.99s (± 0.28%)13.92s (± 0.16%)-0.07s (- 0.50%)13.87s13.96s
Compiler-Unions - node (v14.15.1, x64)
Memory used191,057k (± 0.02%)191,070k (± 0.02%)+13k (+ 0.01%)191,022k191,181k
Parse Time0.80s (± 0.77%)0.79s (± 0.63%)-0.00s (- 0.50%)0.78s0.80s
Bind Time0.55s (± 0.66%)0.55s (± 0.54%)-0.00s (- 0.36%)0.55s0.56s
Check Time6.99s (± 0.54%)6.95s (± 0.44%)-0.04s (- 0.56%)6.90s7.03s
Emit Time2.54s (± 0.65%)2.53s (± 0.69%)-0.01s (- 0.39%)2.49s2.56s
Total Time10.89s (± 0.35%)10.83s (± 0.36%)-0.05s (- 0.49%)10.76s10.92s
Monaco - node (v14.15.1, x64)
Memory used338,065k (± 0.01%)338,066k (± 0.01%)+0k (+ 0.00%)338,029k338,103k
Parse Time1.56s (± 0.62%)1.56s (± 0.63%)+0.00s (+ 0.19%)1.54s1.59s
Bind Time0.75s (± 0.64%)0.74s (± 0.49%)-0.00s (- 0.40%)0.74s0.75s
Check Time4.94s (± 0.55%)4.95s (± 0.36%)+0.01s (+ 0.20%)4.91s4.99s
Emit Time3.17s (± 0.67%)3.15s (± 0.50%)-0.02s (- 0.60%)3.12s3.18s
Total Time10.41s (± 0.39%)10.40s (± 0.29%)-0.01s (- 0.10%)10.34s10.45s
TFS - node (v14.15.1, x64)
Memory used292,237k (± 0.01%)292,247k (± 0.01%)+11k (+ 0.00%)292,206k292,308k
Parse Time1.25s (± 1.06%)1.24s (± 0.88%)-0.01s (- 0.88%)1.22s1.27s
Bind Time0.72s (± 1.11%)0.71s (± 0.91%)-0.01s (- 0.84%)0.70s0.72s
Check Time4.59s (± 0.37%)4.56s (± 0.44%)-0.03s (- 0.70%)4.53s4.61s
Emit Time3.26s (± 0.43%)3.24s (± 0.48%)-0.03s (- 0.80%)3.20s3.27s
Total Time9.83s (± 0.36%)9.75s (± 0.34%)-0.07s (- 0.73%)9.70s9.83s
material-ui - node (v14.15.1, x64)
Memory used477,872k (± 0.01%)477,851k (± 0.05%)-22k (- 0.00%)476,829k478,026k
Parse Time2.06s (± 0.74%)2.05s (± 0.70%)-0.01s (- 0.49%)2.02s2.08s
Bind Time0.69s (± 0.99%)0.69s (± 0.71%)-0.00s (- 0.14%)0.68s0.70s
Check Time12.86s (± 0.48%)12.93s (± 0.86%)+0.07s (+ 0.55%)12.80s13.26s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time15.62s (± 0.41%)15.68s (± 0.76%)+0.05s (+ 0.35%)15.51s16.02s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-198-generic
Architecturex64
Available Memory16 GB
Available Memory5 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
BenchmarkNameIterations
Current4308610
Baselinemaster10

Developer Information:

Download Benchmark

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser)
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..43086

Metricmaster43086DeltaBestWorst
Angular - node (v14.15.1, x64)
Memory used324,919k (± 0.01%)324,883k (± 0.01%)-36k (- 0.01%)324,859k324,934k
Parse Time1.92s (± 0.91%)1.89s (± 0.34%)-0.03s (- 1.41%)1.88s1.91s
Bind Time0.85s (± 0.52%)0.85s (± 0.55%)-0.00s (- 0.23%)0.84s0.86s
Check Time4.91s (± 0.43%)4.90s (± 0.37%)-0.01s (- 0.24%)4.87s4.96s
Emit Time6.30s (± 0.42%)6.24s (± 0.37%)-0.06s (- 0.97%)6.19s6.31s
Total Time13.99s (± 0.28%)13.88s (± 0.18%)-0.11s (- 0.76%)13.83s13.95s
Compiler-Unions - node (v14.15.1, x64)
Memory used191,057k (± 0.02%)191,048k (± 0.02%)-9k (- 0.00%)190,953k191,126k
Parse Time0.80s (± 0.77%)0.79s (± 0.60%)-0.01s (- 0.75%)0.79s0.81s
Bind Time0.55s (± 0.66%)0.55s (± 0.40%)-0.00s (- 0.54%)0.55s0.56s
Check Time6.99s (± 0.54%)6.97s (± 0.45%)-0.02s (- 0.30%)6.90s7.02s
Emit Time2.54s (± 0.65%)2.55s (± 1.54%)+0.01s (+ 0.20%)2.49s2.69s
Total Time10.89s (± 0.35%)10.86s (± 0.29%)-0.03s (- 0.23%)10.79s10.93s
Monaco - node (v14.15.1, x64)
Memory used338,065k (± 0.01%)338,052k (± 0.01%)-14k (- 0.00%)337,994k338,086k
Parse Time1.56s (± 0.62%)1.55s (± 0.54%)-0.01s (- 0.51%)1.53s1.57s
Bind Time0.75s (± 0.64%)0.75s (± 0.67%)-0.00s (- 0.27%)0.74s0.76s
Check Time4.94s (± 0.55%)4.94s (± 0.48%)-0.00s (- 0.04%)4.90s5.00s
Emit Time3.17s (± 0.67%)3.15s (± 0.90%)-0.02s (- 0.63%)3.07s3.19s
Total Time10.41s (± 0.39%)10.38s (± 0.38%)-0.03s (- 0.30%)10.30s10.47s
TFS - node (v14.15.1, x64)
Memory used292,237k (± 0.01%)292,264k (± 0.01%)+28k (+ 0.01%)292,217k292,299k
Parse Time1.25s (± 1.06%)1.25s (± 1.24%)-0.00s (- 0.08%)1.22s1.29s
Bind Time0.72s (± 1.11%)0.71s (± 0.78%)-0.01s (- 0.84%)0.70s0.73s
Check Time4.59s (± 0.37%)4.58s (± 0.46%)-0.02s (- 0.35%)4.52s4.62s
Emit Time3.26s (± 0.43%)3.25s (± 0.40%)-0.02s (- 0.52%)3.21s3.28s
Total Time9.83s (± 0.36%)9.78s (± 0.32%)-0.04s (- 0.45%)9.73s9.85s
material-ui - node (v14.15.1, x64)
Memory used477,872k (± 0.01%)477,826k (± 0.05%)-46k (- 0.01%)476,793k477,984k
Parse Time2.06s (± 0.74%)2.05s (± 0.57%)-0.00s (- 0.15%)2.04s2.09s
Bind Time0.69s (± 0.99%)0.70s (± 0.80%)+0.00s (+ 0.43%)0.69s0.71s
Check Time12.86s (± 0.48%)12.89s (± 0.64%)+0.03s (+ 0.20%)12.79s13.17s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time15.62s (± 0.41%)15.64s (± 0.53%)+0.02s (+ 0.14%)15.53s15.93s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-198-generic
Architecturex64
Available Memory16 GB
Available Memory5 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)
BenchmarkNameIterations
Current4308610
Baselinemaster10

Developer Information:

Download Benchmark

@typescript-botTypeScript Bot (typescript-bot) mentioned this pull request Mar 5, 2021
Open
@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

Closed
@ahejlsbergAnders Hejlsberg (ahejlsberg) mentioned this pull request Jun 13, 2021
Closed
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@RyanCavanaughRyan Cavanaugh (RyanCavanaugh)RyanCavanaugh approved these changes
@DanielRosenwasserDaniel Rosenwasser (DanielRosenwasser)DanielRosenwasser approved these changes
@weswighamWesley Wigham (weswigham)weswigham approved these changes
@andrewbranchAndrew Branch (andrewbranch)Awaiting requested review from andrewbranch

Assignees

@ahejlsbergAnders Hejlsberg (ahejlsberg)

Labels

Author: TeamFor Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Object spread incorrectly introduces undefined value type in TS 4.2

5 participants

@ahejlsberg@DanielRosenwasser@typescript-bot@weswigham@RyanCavanaugh