facebook / react / 13776

86%

Build Type

Pull #8117

travis-ci

Committed by Christopher Chedeau

Commit Message

Attach event listeners at the root of the tree instead of document

Context: we are investigating using React in order to build both Atom core components and Atom plugins. Work have been done last year to make sure that multiple React instances in the same app and this is working well. The only remaining issue is the fact that e.stopPropagation doesn't work as intended when there's two React trees that are nested. The reason is that both event listeners are added to the root of the tree and therefore do not cancel each others. The suggested solution is to attach event listeners at the root of the React tree.

To understand how this solves the problem, let's assume that we have OuterComponent which is running React version A and InnerComponent that is running version 2. The inner component attaches the event listener at the top of the inner tree using bubble phase and the outer component at the root of the outer component.

When there's a click event on the innerComponent, the inner version of React will be notified first because it's deeper in the tree, which will dispatch the event through the innerComponent hierarchy and eventually something will call React e.stopPropagation(), which will call the DOM e.stopPropagation(), so that the outer version of React will never be notified.

Test Plan:
- Have the changes of this revision
- `yarn build` to generate react.js and react-dom.js
- Copy react.js and react-dom.js into react2.js and react-dom2.js
- Change the global assignation of React and ReactDOM inside of *2.js to React2 and ReactDOM2
- Build a test case with two nested components and the inner one calling e.stopPropagation
- http://fooo.fr/~vjeux/fb/vjeux-test/test-event-boundary.html
- Make sure it doesn't trigger the outer one.
- Revert the changes and make sure that clicking on the inner one triggers both events to fire

Important Note:
I don't fully understand the implication of this changes around performance and potential side-effects that c... (continued)

Pull Request Pull Request #8117: Attach event listeners at the root of the tree instead of document

Run Details

3 of 3 new or added lines in 2 files covered. (100.0%)

8124 of 9172 relevant lines covered (88.57%)

3133.22 hits per line