Event propagation test suite #19483
Event propagation test suite #19483
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
| }); | ||
| jest.isolateModules(() => { | ||
| InnerReactDOM = require('react-dom'); | ||
| }); |
There was a problem hiding this comment.
This is to ensure we're actually testing the interop scenario.
| }); | ||
| }); | ||
|
|
||
| it('onKeyPress', () => { |
There was a problem hiding this comment.
Each event has a separate test case so that it's easy to "focus" on it.
| }); | ||
|
|
||
| it('onKeyPress', () => { | ||
| testNativeBubblingEvent({ |
There was a problem hiding this comment.
However, the actual tests are a sequence of several in each case. I didn't want to repeat because we want consistent behavior across events in the same group. So we call a helper function that runs all tests. If you want to run one case in particular you can always comment out the calls inside of that function.
|
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 34bcdb2:
|
| testNativeBubblingEvent({ | ||
| type: 'input', | ||
| reactEvent: 'onFocus', | ||
| nativeEvent: 'focusin', |
There was a problem hiding this comment.
This one is interesting. It's in the bubbling category because it's effectively an alias for the browser focusin which does bubble. So it's React event name that's a misnomer.
There was a problem hiding this comment.
This is really good. Great work Dan.
In the future, we can added some extended tests around portals too, as those have proven to also be problematic areas around React, especially in regards to how event propagation works (and if it even should work for certain events).
We've been making a series of changes to the event system with the goal of improving interop in the scenarios where a React app is nested into another React app — or even a non-React app. In particular, because React used event delegation to the document,
stopPropagation()did not correctly work between two Reacts, as events would not bubble between them.@trueadm has landed most of the work related to this, but we don't have a very good regression suite for events. We also don't have a place where all the behaviors are documented. So I'm adding a test suite that verifies the expected propagation scenarios, both within and across isolated React roots.
This is a suite of jsdom tests so that we notice regressions early on. Initially, I wrote it as a browser fixture (and it's not hard to convert back), but I found that it wasn't particularly useful since I was emulating the events anyway, and we know well which ones bubble (or don't bubble) in the browsers. So I just hardcoded that in the test.
The suite doesn't include all events, but it includes
all kinds of events(edit: there's a few more I need to add) for which we have different behavior. We can add more concrete events to it but I don't think this is a blocker for merging the suite.Note that the suite would not pass in React 16. It documents the current behavior, with tests verifying that cross-root propagation works as well as it can. React 16 did not do a very good job at that.
I will add some other comments inline.