[react-interactions] Upgrade passive event listeners to active listeners #17513
Conversation
This file contains 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
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 689ed6d:
|
Details of bundled changes.Comparing: 54f6673...689ed6d react-dom
ReactDOM: size: 0.0%, gzip: -0.0% Size changes (experimental) |
Details of bundled changes.Comparing: 54f6673...689ed6d react-dom
ReactDOM: size: 0.0%, gzip: 🔺+0.1% Size changes (stable) |
…listeners WIP WIP FIX typos
f510cc6 to
d22e0af
Compare
necolas
approved these changes
Dec 4, 2019
trueadm
added a commit
to trueadm/react
that referenced
this pull request
Dec 4, 2019
sebmarkbage
added a commit
to sebmarkbage/react
that referenced
this pull request
Jan 30, 2020
This effectively reverts part of facebook#17513
sebmarkbage
added a commit
to sebmarkbage/react
that referenced
this pull request
Jan 30, 2020
This effectively reverts part of facebook#17513
sebmarkbage
added a commit
that referenced
this pull request
Jan 30, 2020
* Add test of Event Replaying using Flare * Fix Event Replaying in Flare by Eagerly Adding Active Listeners This effectively reverts part of #17513
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
One of the long standing things we've wanted to do when working on React Flare, was to be able to upgrade passive event listeners to active event listeners where possible. This removes the need to have two separate event listeners attached to the document at the same time.
If we first attach an active listener of a given type, and then later try to add a passive listener of the same type, we don't both – instead we handle the logic for both active and passive events inside the same active event listener. Furthermore, if we first attach a passive event for a given type, and then later try to add an active listener of the same type, we first remove the passive listener from the DOM and then we add the active listener to the DOM.
The logic in the responder system might seem a little convoluted because of how responders target/root events are designed. Thankfully, this won't be the case with the Listener API and we use a much simpler control flow when dealing with events with that API.
I also remove the responder logic for
passiveSupportedas it no longer felt relevant given this change, plus it was never used in any event responders. To enable the changes, we change the Listener Set to a Listener Map, so we can store the event listener callback we used when setting up the DOM event listener (this enables us to remove it again later).