From ba392590eb1267b0ead16567e20f14ff88d5e5cf Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 9 Oct 2017 19:39:19 +0100 Subject: [PATCH 1/4] Add a regression test for #10906 --- .../__tests__/EnterLeaveEventPlugin-test.js | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js b/src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js index 1540236b8ed..c9bb7863a93 100644 --- a/src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js +++ b/src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js @@ -13,6 +13,7 @@ var EnterLeaveEventPlugin; var React; var ReactDOM; var ReactDOMComponentTree; +var ReactTestUtils; describe('EnterLeaveEventPlugin', () => { beforeEach(() => { @@ -20,6 +21,7 @@ describe('EnterLeaveEventPlugin', () => { React = require('react'); ReactDOM = require('react-dom'); + ReactTestUtils = require('react-dom/test-utils'); // TODO: can we express this test with only public API? ReactDOMComponentTree = require('ReactDOMComponentTree'); EnterLeaveEventPlugin = require('EnterLeaveEventPlugin'); @@ -58,4 +60,38 @@ describe('EnterLeaveEventPlugin', () => { expect(enter.target).toBe(div); expect(enter.relatedTarget).toBe(iframe.contentWindow); }); + + // Regression test for https://github.com/facebook/react/issues/10906. + it('should find the common parent after updates', () => { + let parentEnterCalls = 0; + let childEnterCalls = 0; + let parent = null; + class Parent extends React.Component { + render() { + return ( +
parentEnterCalls++} + ref={node => (parent = node)}> + {this.props.showChild && +
childEnterCalls++} />} +
+ ); + } + } + + const div = document.createElement('div'); + ReactDOM.render(, div); + // The issue only reproduced on insertion during the first update. + ReactDOM.render(, div); + + // Enter from parent into the child. + ReactTestUtils.simulateNativeEventOnNode('topMouseOut', parent, { + target: parent, + relatedTarget: parent.firstChild, + }); + + // Entering a child should fire on the child, not on the parent. + expect(childEnterCalls).toBe(1); + expect(parentEnterCalls).toBe(0); + }); }); From 28b04738c62a72d9378f45cbefc4e48bb72a1be5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 9 Oct 2017 19:57:10 +0100 Subject: [PATCH 2/4] Turn while conditions into breaks without changing the logic This will be easier to follow when we add more code there. --- .../shared/shared/ReactTreeTraversal.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/shared/ReactTreeTraversal.js b/src/renderers/shared/shared/ReactTreeTraversal.js index 8b58d7b10b7..9ac106f3af7 100644 --- a/src/renderers/shared/shared/ReactTreeTraversal.js +++ b/src/renderers/shared/shared/ReactTreeTraversal.js @@ -112,12 +112,24 @@ function traverseTwoPhase(inst, fn, arg) { function traverseEnterLeave(from, to, fn, argFrom, argTo) { var common = from && to ? getLowestCommonAncestor(from, to) : null; var pathFrom = []; - while (from && from !== common) { + while (true) { + if (!from) { + break; + } + if (from === common) { + break; + } pathFrom.push(from); from = getParent(from); } var pathTo = []; - while (to && to !== common) { + while (true) { + if (!to) { + break; + } + if (to === common) { + break; + } pathTo.push(to); to = getParent(to); } From 2e099be61b72434c7c28109d78048cbd4972b926 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 9 Oct 2017 20:01:22 +0100 Subject: [PATCH 3/4] var => const/let So that I can add a block scoped variable. --- src/renderers/shared/shared/ReactTreeTraversal.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/shared/shared/ReactTreeTraversal.js b/src/renderers/shared/shared/ReactTreeTraversal.js index 9ac106f3af7..efe4da50e77 100644 --- a/src/renderers/shared/shared/ReactTreeTraversal.js +++ b/src/renderers/shared/shared/ReactTreeTraversal.js @@ -110,8 +110,8 @@ function traverseTwoPhase(inst, fn, arg) { * "entered" or "left" that element. */ function traverseEnterLeave(from, to, fn, argFrom, argTo) { - var common = from && to ? getLowestCommonAncestor(from, to) : null; - var pathFrom = []; + const common = from && to ? getLowestCommonAncestor(from, to) : null; + const pathFrom = []; while (true) { if (!from) { break; @@ -122,7 +122,7 @@ function traverseEnterLeave(from, to, fn, argFrom, argTo) { pathFrom.push(from); from = getParent(from); } - var pathTo = []; + const pathTo = []; while (true) { if (!to) { break; @@ -133,7 +133,7 @@ function traverseEnterLeave(from, to, fn, argFrom, argTo) { pathTo.push(to); to = getParent(to); } - var i; + let i; for (i = 0; i < pathFrom.length; i++) { fn(pathFrom[i], 'bubbled', argFrom); } From ef6d6fcc8158217acdd41e3ddde4e5b424030782 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 9 Oct 2017 20:01:49 +0100 Subject: [PATCH 4/4] Check alternates when comparing to the common ancestor This is the actual bugfix. --- src/renderers/shared/shared/ReactTreeTraversal.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/renderers/shared/shared/ReactTreeTraversal.js b/src/renderers/shared/shared/ReactTreeTraversal.js index efe4da50e77..d483881530b 100644 --- a/src/renderers/shared/shared/ReactTreeTraversal.js +++ b/src/renderers/shared/shared/ReactTreeTraversal.js @@ -119,6 +119,10 @@ function traverseEnterLeave(from, to, fn, argFrom, argTo) { if (from === common) { break; } + const alternate = from.alternate; + if (alternate !== null && alternate === common) { + break; + } pathFrom.push(from); from = getParent(from); } @@ -130,6 +134,10 @@ function traverseEnterLeave(from, to, fn, argFrom, argTo) { if (to === common) { break; } + const alternate = to.alternate; + if (alternate !== null && alternate === common) { + break; + } pathTo.push(to); to = getParent(to); }