[Fiber] Read Event Handlers from the "Current" Fiber #8491
Merged
Conversation
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
When we perform an update to the event handler we properly update the immediate Fiber pointer of a child to be the current. However, when we bubble events we use the return pointer which is not guaranteed to point to the current Fiber even if we start from the current. This manifests itself when we bailout in a parent. So I made the tests use a PureComponent to illustrate this scenario. There is already a failing case but I'm adding another one too.
This is unnecessary forwarding since this is no longer injectable. However, I'd like to use the injectable getInstanceFromNode from TreeTraversal so this avoids a cyclic dependency.
This fixes the issue where using the .return pointer isn't guaranteed to return the current Fiber so we might read the wrong props when we try to get the current event.
Collaborator
Author
|
friendlyping |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 ensures that we read the event handlers from the current Fiber since the
target.returnpointer is not sufficient even iftargetis the current Fiber.I'm not super happy with this solution but I don't have a better idea atm.