From e16703e6c7776ff39e30c448df075f48bdbe08ea Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 11 May 2020 22:09:52 +0100 Subject: [PATCH] Modern Event System: revise ancestor logic (#18886) --- .../src/events/DOMModernPluginEventSystem.js | 31 ++++--- ...OMModernPluginEventSystem-test.internal.js | 89 +++++++++++++++++++ 2 files changed, 107 insertions(+), 13 deletions(-) diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js index 095e1b6ce8..03ecd50ade 100644 --- a/packages/react-dom/src/events/DOMModernPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMModernPluginEventSystem.js @@ -417,13 +417,13 @@ export function dispatchEventForPluginEventSystem( // sub-tree for that root and make that our ancestor instance. let node = targetInst; - while (true) { + mainLoop: while (true) { if (node === null) { return; } const nodeTag = node.tag; if (nodeTag === HostRoot || nodeTag === HostPortal) { - const container = node.stateNode.containerInfo; + let container = node.stateNode.containerInfo; if (isMatchingRootContainer(container, targetContainerNode)) { break; } @@ -449,18 +449,23 @@ export function dispatchEventForPluginEventSystem( grandNode = grandNode.return; } } - const parentSubtreeInst = getClosestInstanceFromNode(container); - if (parentSubtreeInst === null) { - return; + // Now we need to find it's corresponding host fiber in the other + // tree. To do this we can use getClosestInstanceFromNode, but we + // need to validate that the fiber is a host instance, otherwise + // we need to traverse up through the DOM till we find the correct + // node that is from the other tree. + while (container !== null) { + const parentNode = getClosestInstanceFromNode(container); + if (parentNode === null) { + return; + } + const parentTag = parentNode.tag; + if (parentTag === HostComponent || parentTag === HostText) { + node = ancestorInst = parentNode; + continue mainLoop; + } + container = container.parentNode; } - const parentTag = parentSubtreeInst.tag; - // getClosestInstanceFromNode can return a HostRoot or SuspenseComponent. - // So we need to ensure we only set the ancestor to a HostComponent or HostText. - if (parentTag === HostComponent || parentTag === HostText) { - ancestorInst = parentSubtreeInst; - } - node = parentSubtreeInst; - continue; } node = node.return; } diff --git a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js index 538353fff4..a690b574e9 100644 --- a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js @@ -224,6 +224,95 @@ describe('DOMModernPluginEventSystem', () => { expect(log[5]).toEqual(['bubble', buttonElement]); }); + it('handle propagation of click events between disjointed roots #2', () => { + const buttonRef = React.createRef(); + const button2Ref = React.createRef(); + const divRef = React.createRef(); + const spanRef = React.createRef(); + const log = []; + const onClick = jest.fn(e => log.push(['bubble', e.currentTarget])); + const onClickCapture = jest.fn(e => + log.push(['capture', e.currentTarget]), + ); + + function Child() { + return ( +
+ Click me! +
+ ); + } + + function Parent() { + return ( + + ); + } + + // We make a wrapper with an inner container that we + // render to. So it looks like
+ // We then render to all three: + // - container + // - parentContainer + // - childContainer + + const parentContainer = document.createElement('div'); + const childContainer = document.createElement('div'); + + ReactDOM.render(, container); + ReactDOM.render(, parentContainer); + ReactDOM.render(, childContainer); + + parentContainer.appendChild(childContainer); + spanRef.current.appendChild(parentContainer); + + // Inside + const buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + expect(onClick).toHaveBeenCalledTimes(1); + expect(onClickCapture).toHaveBeenCalledTimes(1); + expect(log[0]).toEqual(['capture', buttonElement]); + expect(log[1]).toEqual(['bubble', buttonElement]); + + // Inside + const divElement = divRef.current; + dispatchClickEvent(divElement); + expect(onClick).toHaveBeenCalledTimes(3); + expect(onClickCapture).toHaveBeenCalledTimes(3); + expect(log[2]).toEqual(['capture', divElement]); + expect(log[3]).toEqual(['bubble', divElement]); + expect(log[4]).toEqual(['capture', buttonElement]); + expect(log[5]).toEqual(['bubble', buttonElement]); + + // Inside + const buttonElement2 = button2Ref.current; + dispatchClickEvent(buttonElement2); + expect(onClick).toHaveBeenCalledTimes(5); + expect(onClickCapture).toHaveBeenCalledTimes(5); + expect(log[6]).toEqual(['capture', buttonElement2]); + expect(log[7]).toEqual(['bubble', buttonElement2]); + expect(log[8]).toEqual(['capture', buttonElement]); + expect(log[9]).toEqual(['bubble', buttonElement]); + }); + it('handle propagation of click events between disjointed comment roots', () => { const buttonRef = React.createRef(); const divRef = React.createRef();