Refactor side effect handling for property interactions#4522
Merged
lukastaegert merged 13 commits intomasterfrom Jun 7, 2022
Merged
Refactor side effect handling for property interactions#4522lukastaegert merged 13 commits intomasterfrom
lukastaegert merged 13 commits intomasterfrom
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#refactor-interactionsor 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
pos777
pushed a commit
to pos777/rollup
that referenced
this pull request
Jun 18, 2022
* 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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
hasEffectsOnInteractionAtPaththat 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
thisArgthat 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,argsis 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
thisvalue when called in updated expressions or for-in/of loops Previous version REPL → FIxed REPL