fix(51521): Using a of property declared after an initializing constructor triggers an assertion failure in JS#51524
Conversation
|
@typescript-bot pack this I feel odd about this change - not binding feels like the wrong thing. The attempt to do JS inference in the first place is the wrong thing. |
|
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 9dc7b92. You can monitor the build here. |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
In fact, I think this causes other issues. If you open the built playground, switch to a JS file, paste in the code, and request diagnostics, you will get the following stack trace. Fuller StackCan you make your test request errors as well? |
|
Hmm, maybe #52321 is better if not binding is not preferable? (Tomorrow I'll grab this test and see) |
|
A playground should be ready shortly to try out. If you get to this before Jake does, feel free to cherry-pick the commit. |
… crashes in checker
jakebailey
left a comment
There was a problem hiding this comment.
Rechecked this against the tsreplay for the babel example and it does pass. This fix seems to be the best one?
|
@jakebailey What is |
See the expando here, for example: #52317 (comment) |
|
@typescript-bot perf test this faster |
|
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at 7cd7628. You can monitor the build here. Update: The results are in! |
|
@typescript-bot user test tsserver |
|
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite (tsserver) on this PR at 7cd7628. You can monitor the build here. Update: The results are in! |
|
Heya @DanielRosenwasser, I've started to run the diff-based user code test suite (tsserver) on this PR at 7cd7628. You can monitor the build here. Update: The results are in! |
DanielRosenwasser
left a comment
There was a problem hiding this comment.
Approved pending reasonable perf and test results.
|
@DanielRosenwasser Here are the results of running the user test suite comparing Everything looks good! |
|
@DanielRosenwasser Here they are:Comparison Report - main..51524
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Perf looks good; merging. |
|
@DanielRosenwasser Here are the results of running the top-repos suite comparing Everything looks good! |
Fixes #51521