fix: Deoptimize the proxied object if its property is reassigned in the handler functions#5713
fix: Deoptimize the proxied object if its property is reassigned in the handler functions#5713lukastaegert merged 4 commits intomasterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#fix-5709Notice: Ensure you have installed the latest stable Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust. or load it into the REPL: |
Performance report
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5713 +/- ##
=======================================
Coverage 99.01% 99.02%
=======================================
Files 257 258 +1
Lines 8055 8062 +7
Branches 1358 1359 +1
=======================================
+ Hits 7976 7983 +7
Misses 52 52
Partials 27 27 ☔ View full report in Codecov by Sentry. |
lukastaegert
left a comment
There was a problem hiding this comment.
Nice, and thank you for looking into this so quickly. The solution looks good to me!
For fixing the circular dependency issues, I added src/ast/utils/identifyNode.ts file. I think we need to sort out the dependencies of some of the current ast files later, especially where instanceof is called.
I actually have a permanent fix for this mostly prepared for Rollup 5 on a branch. Basically what I want to do is auto-generated the AST types from the same files that we use to generate the AST converters so that they are perfectly in sync. This will also include the internally used types in the AST classes. This will be based on union types so that then, doing a string comparison for Node.type would actually narrow the TypeScript type. Thus, we can replace all instanceof checks with .type comparisons without the need to define manual type guards any more.
|
This PR has been released as part of rollup@4.24.4. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
resolved #5709
Description
For fixing the circular dependency issues, I added
src/ast/utils/identifyNode.tsfile. I think we need to sort out the dependencies of some of the current ast files later, especially whereinstanceofis called.