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
Refactor side effect handling for property interactions #4522
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#refactor-interactions or load it into the REPL: |
Codecov Report
@@ Coverage Diff @@
## master #4522 +/- ##
==========================================
+ Coverage 98.76% 98.89% +0.12%
==========================================
Files 209 208 -1
Lines 7395 7332 -63
Branches 2108 2094 -14
==========================================
- Hits 7304 7251 -53
+ Misses 32 26 -6
+ Partials 59 55 -4
Continue to review full report at Codecov.
|
c43c3df
to
605c701
Compare
* Rename event -> interaction * Refactor MemberExpression assignment logic to prepare for new interactions * Use objects for interactions * Add additional interaction parameters * Pick "this"-argument from interaction * Use interaction for getReturnExpression * Replace dedicated methods with hasEffectsOnInteractionAtPath * Fix accessor handling for loops and updated expressions * Simplify assignment handling * Change assignment to use args property * Simplify ObjectEntity effect handling * Cache remaining interactions that may be cached * Improve coverage
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
This will refactor and simplify the mechanism for checking property side effects (read, write, call) to use a single function
hasEffectsOnInteractionAtPath
that uses "interaction" objects as parameters that are also used bydeoptimizeThisOnInteractionAtPath
(formerly "eventAtPath).These interaction objects have a type to determine if it is a read, write or call. They also have a
thisArg
that is used for deoptimizations but may also be used for other purposes in the future. Besides depending on the interaction type, they may also specifyargs
. For writes,args
is an array with a single element to make it easy for setters to convert writes to calls.While this is mostly a refactoring, it will hopefully make the algorithm slightly more approachable, but it will more importantly lay the ground work for some future improvements.
Also, some subtle bugs were fixed as setters did not properly deoptimize their
this
value when called in updated expressions or for-in/of loops Previous version REPL → FIxed REPL