Skip to content

Fix double error on invalid delete of readonly property #55449

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

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

shinichy
Copy link
Contributor

Fixes #53082. This PR added a unit test based on #53961.

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Aug 21, 2023
@shinichy
Copy link
Contributor Author

@microsoft-github-policy-service agree

@jakebailey
Copy link
Member

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 2023

Heya @jakebailey, I've started to run the diff-based top-repos suite (tsserver) on this PR at 43590df. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 21, 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/156751/artifacts?artifactName=tgz&fileId=9AA462E5F3F96A2FEC2B26B1A0AAC53F049DE420193ECC00029DE74F602DD0CA02&fileName=/typescript-5.3.0-insiders.20230821.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/[email protected]".;

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

There were infrastructure failures potentially unrelated to your change:

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

Otherwise...

Something interesting changed - please have a look.

Details

rxjs-src

/mnt/ts_downloads/rxjs-src/build.sh

  • [NEW] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55449/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
  • [MISSING] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)

@typescript-bot
Copy link
Collaborator

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

Here they are:

Compiler

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 300,363k (± 0.00%) 300,358k (± 0.01%) ~ 300,341k 300,392k p=0.260 n=6
Parse Time 3.02s (± 0.17%) 3.02s (± 0.28%) ~ 3.00s 3.02s p=0.070 n=6
Bind Time 0.93s (± 0.00%) 0.93s (± 0.00%) ~ 0.93s 0.93s p=1.000 n=6
Check Time 9.49s (± 0.30%) 9.49s (± 0.36%) ~ 9.44s 9.52s p=0.870 n=6
Emit Time 7.63s (± 0.34%) 7.64s (± 0.43%) ~ 7.59s 7.68s p=0.366 n=6
Total Time 21.08s (± 0.19%) 21.08s (± 0.29%) ~ 20.97s 21.14s p=1.000 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,886k (± 0.01%) 193,871k (± 0.02%) ~ 193,827k 193,926k p=0.378 n=6
Parse Time 1.57s (± 1.30%) 1.56s (± 0.99%) ~ 1.55s 1.58s p=0.417 n=6
Bind Time 0.79s (± 0.00%) 0.79s (± 0.00%) ~ 0.79s 0.79s p=1.000 n=6
Check Time 9.94s (± 0.23%) 9.94s (± 0.34%) ~ 9.91s 10.00s p=0.627 n=6
Emit Time 2.74s (± 0.15%) 2.74s (± 0.27%) ~ 2.73s 2.75s p=0.389 n=6
Total Time 15.04s (± 0.14%) 15.03s (± 0.23%) ~ 14.99s 15.08s p=0.808 n=6
Monaco - node (v16.17.1, x64)
Memory used 347,125k (± 0.00%) 347,129k (± 0.00%) ~ 347,124k 347,147k p=0.421 n=6
Parse Time 2.69s (± 0.33%) 2.68s (± 0.30%) ~ 2.67s 2.69s p=0.270 n=6
Bind Time 0.99s (± 0.00%) 0.99s (± 0.00%) ~ 0.99s 0.99s p=1.000 n=6
Check Time 7.92s (± 0.25%) 7.94s (± 0.28%) ~ 7.91s 7.97s p=0.252 n=6
Emit Time 4.25s (± 0.41%) 4.27s (± 0.43%) ~ 4.24s 4.29s p=0.247 n=6
Total Time 15.85s (± 0.17%) 15.88s (± 0.19%) ~ 15.84s 15.92s p=0.144 n=6
TFS - node (v16.17.1, x64)
Memory used 301,137k (± 0.00%) 301,144k (± 0.01%) ~ 301,111k 301,157k p=0.149 n=6
Parse Time 2.18s (± 0.41%) 2.18s (± 0.58%) ~ 2.16s 2.20s p=1.000 n=6
Bind Time 1.11s (± 0.37%) 1.11s (± 0.37%) ~ 1.10s 1.11s p=0.218 n=6
Check Time 7.24s (± 0.38%) 7.24s (± 0.23%) ~ 7.21s 7.25s p=1.000 n=6
Emit Time 3.96s (± 0.26%) 3.97s (± 0.82%) ~ 3.92s 4.01s p=0.360 n=6
Total Time 14.48s (± 0.26%) 14.50s (± 0.18%) ~ 14.45s 14.53s p=0.419 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,480k (± 0.01%) 479,475k (± 0.01%) ~ 479,423k 479,536k p=0.936 n=6
Parse Time 3.15s (± 0.20%) 3.15s (± 0.17%) ~ 3.15s 3.16s p=0.201 n=6
Bind Time 0.91s (± 0.00%) 0.91s (± 0.00%) ~ 0.91s 0.91s p=1.000 n=6
Check Time 17.99s (± 0.40%) 17.92s (± 0.42%) ~ 17.83s 18.01s p=0.226 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.04s (± 0.32%) 21.99s (± 0.35%) ~ 21.89s 22.08s p=0.296 n=6
xstate - node (v16.17.1, x64)
Memory used 542,845k (± 0.01%) 542,826k (± 0.02%) ~ 542,766k 542,988k p=0.298 n=6
Parse Time 3.70s (± 0.30%) 3.70s (± 0.22%) ~ 3.68s 3.70s p=1.000 n=6
Bind Time 1.38s (± 4.31%) 1.38s (± 4.31%) ~ 1.34s 1.46s p=1.000 n=6
Check Time 3.29s (± 2.59%) 3.29s (± 2.51%) ~ 3.18s 3.37s p=0.871 n=6
Emit Time 0.08s (± 0.00%) 0.08s (± 4.99%) ~ 0.08s 0.09s p=0.405 n=6
Total Time 8.46s (± 0.31%) 8.45s (± 0.32%) ~ 8.41s 8.49s p=0.572 n=6
System info unknown
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 pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,492ms (± 0.23%) 2,489ms (± 0.11%) ~ 2,484ms 2,491ms p=0.574 n=6
Req 2 - geterr 5,960ms (± 0.40%) 5,947ms (± 0.36%) ~ 5,924ms 5,984ms p=0.378 n=6
Req 3 - references 344ms (± 1.35%) 344ms (± 0.49%) ~ 342ms 346ms p=0.366 n=6
Req 4 - navto 279ms (± 1.01%) 277ms (± 0.27%) ~ 276ms 278ms p=0.453 n=6
Req 5 - completionInfo count 1,356 (± 0.00%) 1,356 (± 0.00%) ~ 1,356 1,356 p=1.000 n=6
Req 5 - completionInfo 82ms (± 7.63%) 87ms (± 6.73%) ~ 81ms 93ms p=0.145 n=6
CompilerTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,629ms (± 1.09%) 2,620ms (± 0.74%) ~ 2,591ms 2,642ms p=0.810 n=6
Req 2 - geterr 4,752ms (± 0.15%) 4,743ms (± 0.30%) ~ 4,723ms 4,765ms p=0.173 n=6
Req 3 - references 350ms (± 0.12%) 350ms (± 0.23%) ~ 350ms 352ms p=0.218 n=6
Req 4 - navto 269ms (± 0.19%) 269ms (± 0.15%) ~ 269ms 270ms p=0.595 n=6
Req 5 - completionInfo count 1,518 (± 0.00%) 1,518 (± 0.00%) ~ 1,518 1,518 p=1.000 n=6
Req 5 - completionInfo 79ms (± 0.00%) 79ms (± 0.52%) ~ 78ms 79ms p=0.405 n=6
xstateTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,714ms (± 0.09%) 2,716ms (± 0.15%) ~ 2,711ms 2,721ms p=0.171 n=6
Req 2 - geterr 1,934ms (± 1.91%) 1,940ms (± 1.04%) ~ 1,900ms 1,952ms p=0.334 n=6
Req 3 - references 135ms (± 7.87%) 137ms (± 2.95%) ~ 132ms 142ms p=0.625 n=6
Req 4 - navto 353ms (± 0.43%) 353ms (± 0.43%) ~ 352ms 355ms p=0.410 n=6
Req 5 - completionInfo count 2,071 (± 0.00%) 2,071 (± 0.00%) ~ 2,071 2,071 p=1.000 n=6
Req 5 - completionInfo 319ms (± 2.42%) 322ms (± 2.44%) ~ 314ms 332ms p=0.573 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • CompilerTSServer - node (v16.17.1, x64)
  • Compiler-UnionsTSServer - node (v16.17.1, x64)
  • xstateTSServer - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v16.17.1, x64)
Execution time 155.63ms (± 0.20%) 155.43ms (± 0.15%) -0.20ms (- 0.13%) 154.49ms 158.44ms p=0.000 n=600
tsserver-startup - node (v16.17.1, x64)
Execution time 230.95ms (± 0.13%) 230.76ms (± 0.13%) -0.19ms (- 0.08%) 229.63ms 235.56ms p=0.000 n=600
tsserverlibrary-startup - node (v16.17.1, x64)
Execution time 234.90ms (± 0.11%) 236.07ms (± 0.15%) +1.17ms (+ 0.50%) 234.63ms 242.34ms p=0.000 n=600
typescript-startup - node (v16.17.1, x64)
Execution time 235.87ms (± 0.15%) 235.77ms (± 0.12%) -0.10ms (- 0.04%) 234.23ms 240.54ms p=0.004 n=600
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • tsc-startup - node (v16.17.1, x64)
  • tsserver-startup - node (v16.17.1, x64)
  • tsserverlibrary-startup - node (v16.17.1, x64)
  • typescript-startup - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

1 similar comment
@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@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.

@shinichy shinichy requested a review from jakebailey August 22, 2023 05:40
@jakebailey jakebailey merged commit 98d7e0b into microsoft:main Aug 24, 2023
@shinichy shinichy deleted the 53082_double_error_on_delete branch August 25, 2023 03:41
snovader pushed a commit to EG-A-S/TypeScript that referenced this pull request Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Double error on invalid delete of readonly property
3 participants