Skip to content

Exclude special index signature rule from strict subtype relation - #53388

Merged
Anders Hejlsberg (ahejlsberg) merged 4 commits into
mainfrom
fix53379
Mar 23, 2023
Merged

Exclude special index signature rule from strict subtype relation#53388
Anders Hejlsberg (ahejlsberg) merged 4 commits into
mainfrom
fix53379

Conversation

@ahejlsberg

Copy link
Copy Markdown
Member

Fixes#53379.

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

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

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

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

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

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

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 20, 2023

Copy link
Copy Markdown
Contributor

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

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 20, 2023

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 ae63c73. You can monitor the build here.

Update: The results are in!

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg) Here are the results of running the user test suite comparing main and refs/pull/53388/merge:

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Anders Hejlsberg (@ahejlsberg), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@typescript-bot

Copy link
Copy Markdown
Contributor

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

Here they are:

Comparison Report - main..53388

Metricmain53388DeltaBestWorstp-value
Angular - node (v16.17.1, x64)
Memory used361,070k (± 0.01%)361,067k (± 0.01%)~361,029k361,111kp=1.000 n=6
Parse Time3.54s (± 0.70%)3.51s (± 0.49%)-0.03s (- 0.94%)3.49s3.53sp=0.018 n=6
Bind Time1.18s (± 0.44%)1.18s (± 0.44%)~1.18s1.19sp=1.000 n=6
Check Time9.48s (± 0.30%)9.46s (± 0.33%)~9.42s9.51sp=0.252 n=6
Emit Time7.89s (± 0.49%)7.94s (± 0.72%)~7.88s8.04sp=0.171 n=6
Total Time22.10s (± 0.17%)22.09s (± 0.29%)~22.00s22.19sp=0.627 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used192,554k (± 0.02%)192,500k (± 0.02%)~192,422k192,532kp=0.078 n=6
Parse Time1.59s (± 0.40%)1.60s (± 0.85%)~1.58s1.62sp=0.388 n=6
Bind Time0.82s (± 0.50%)0.82s (± 0.50%)~0.82s0.83sp=1.000 n=6
Check Time10.12s (± 0.35%)10.19s (± 0.58%)+0.07s (+ 0.64%)10.11s10.29sp=0.029 n=6
Emit Time2.98s (± 1.01%)2.99s (± 0.62%)~2.97s3.02sp=0.686 n=6
Total Time15.52s (± 0.35%)15.60s (± 0.35%)+0.08s (+ 0.52%)15.52s15.67sp=0.045 n=6
Monaco - node (v16.17.1, x64)
Memory used345,564k (± 0.01%)345,561k (± 0.00%)~345,534k345,576kp=0.688 n=6
Parse Time2.73s (± 0.54%)2.72s (± 0.30%)~2.71s2.73sp=0.677 n=6
Bind Time1.09s (± 0.69%)1.09s (± 1.07%)~1.08s1.11sp=0.796 n=6
Check Time7.70s (± 0.38%)7.71s (± 0.34%)~7.68s7.74sp=0.520 n=6
Emit Time4.45s (± 0.78%)4.46s (± 0.80%)~4.43s4.53sp=0.686 n=6
Total Time15.97s (± 0.48%)15.98s (± 0.34%)~15.93s16.07sp=0.687 n=6
TFS - node (v16.17.1, x64)
Memory used299,781k (± 0.01%)299,809k (± 0.01%)+28k (+ 0.01%)299,769k299,834kp=0.045 n=6
Parse Time2.17s (± 0.47%)2.18s (± 0.79%)~2.16s2.20sp=0.452 n=6
Bind Time1.25s (± 0.84%)1.24s (± 1.35%)~1.22s1.26sp=0.742 n=6
Check Time7.16s (± 0.42%)7.19s (± 0.73%)~7.14s7.26sp=0.629 n=6
Emit Time4.35s (± 1.03%)4.36s (± 0.91%)~4.31s4.42sp=0.687 n=6
Total Time14.93s (± 0.30%)14.97s (± 0.58%)~14.87s15.11sp=0.571 n=6
material-ui - node (v16.17.1, x64)
Memory used476,415k (± 0.01%)476,455k (± 0.01%)~476,380k476,530kp=0.173 n=6
Parse Time3.22s (± 0.42%)3.23s (± 0.36%)~3.22s3.25sp=0.113 n=6
Bind Time0.96s (± 0.54%)0.95s (± 0.88%)~0.95s0.97sp=0.533 n=6
Check Time17.97s (± 0.37%)17.92s (± 0.36%)~17.81s17.99sp=0.261 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time22.14s (± 0.36%)22.10s (± 0.34%)~21.98s22.19sp=0.336 n=6
xstate - node (v16.17.1, x64)
Memory used550,611k (± 0.02%)550,568k (± 0.02%)~550,501k550,756kp=0.336 n=6
Parse Time3.95s (± 0.26%)3.96s (± 0.35%)~3.95s3.98sp=0.086 n=6
Bind Time1.79s (± 0.30%)1.81s (± 0.45%)+0.01s (+ 0.65%)1.80s1.82sp=0.025 n=6
Check Time3.01s (± 0.61%)3.01s (± 0.50%)~2.99s3.03sp=1.000 n=6
Emit Time0.09s (± 0.00%)0.09s (± 0.00%)~0.09s0.09sp=1.000 n=6
Total Time8.85s (± 0.25%)8.88s (± 0.21%)~8.86s8.91sp=0.086 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-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)
BenchmarkNameIterations
Current533886
Baselinemain6

Developer Information:

Download Benchmark

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

Tests and performance are unchanged, except for a new error in typeorm. In that code, FindOptionsWhere<any> is resolved to { [x: string]: any }. The error is caused by the fact that any[] is no longer a strict subtype of { [x: string]: any }, so the type in the false branch becomes any[] | FindOptionsWhere<any>, correctly revealing that where might be a zero length array in the false branch. So, new error seems entirely reasonable.

@DanielRosenwasser

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack this

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the tarball bundle task on this PR at ae63c73. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Hey Daniel Rosenwasser (@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/150747/artifacts?artifactName=tgz&fileId=9EB6E7523EC88284C829E71D1C2E037D04E39129FD4B4A6E11C3B33AB1354E9102&fileName=/typescript-5.1.0-insiders.20230322.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-53388-13".;

@DanielRosenwasser

Copy link
Copy Markdown
Member

Anders Hejlsberg (@ahejlsberg) sounds like what you're saying is that TypeORM is supposed to still handle the 0-length case, and can do so by

Would you be able to add an equivalentish test case to document this?

functionfoo(obj: Record<string,any>|Record<string,any>[]){if(Array.isArray(obj)&&obj.length){for(letkeyinobj){if(obj[key]!==undefined){console.log(obj[key])}}}else{for(letkeyinobj){if(obj[key]!==undefined){console.log(obj[key])}}}}functionbar(obj: Record<string,any>|Record<string,any>[]){if(Array.isArray(obj)){for(letkeyinobj){if(obj[key]!==undefined){console.log(obj[key])}}}else{for(letkeyinobj){if(obj[key]!==undefined){console.log(obj[key])}}}}

@DanielRosenwasser

Copy link
Copy Markdown
Member

Confirmed in the test playground that foo has errors, but bar is fine.

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Mar 22, 2023

Copy link
Copy Markdown
Member

I've added the tests, I'm running a task to update the baselines.

@DanielRosenwasser

Copy link
Copy Markdown
Member

the heck?

@DanielRosenwasser

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) cherry-pick this to release-5.0

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the task to cherry-pick this into release-5.0 on this PR at 37911f8. You can monitor the build here.

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Daniel Rosenwasser (@DanielRosenwasser), I've opened #53445 for you.

TypeScript Bot (typescript-bot) pushed a commit to typescript-bot/TypeScript that referenced this pull request Mar 22, 2023
Component commits:
e2aa5c6 Exclude special index signature rule from strict subtype relation
ae63c73 Add tests
54703cd Add more test cases.
37911f8 Accepted baselines.
Daniel Rosenwasser (DanielRosenwasser) added a commit that referenced this pull request Mar 23, 2023
…e-5.0 (#53445)
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
drivron pushed a commit to scenari/typescript that referenced this pull request Sep 14, 2023
…to release-5.0 (microsoft#53445)
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
@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.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion function & Type guards for Record types broken on 5.0

3 participants

@ahejlsberg@typescript-bot@DanielRosenwasser