{"commit":{"revision":"e16703e6c7776ff39e30c448df075f48bdbe08ea","subject":"Modern Event System: revise ancestor logic (#18886)","authorEmail":"trueadm@users.noreply.github.com","authoredAt":"2020-05-11T22:09:52+01:00","shortRevision":"e16703e6c7","authorName":"Dominic Gannaway"},"body":"","parents":["2b9d7cf65fb5423a972b5dc920a3341b865085bf"],"files":[{"status":"M","path":"packages\/react-dom\/src\/events\/DOMModernPluginEventSystem.js"},{"status":"M","path":"packages\/react-dom\/src\/events\/__tests__\/DOMModernPluginEventSystem-test.internal.js"}],"patch":"diff --git a\/packages\/react-dom\/src\/events\/DOMModernPluginEventSystem.js b\/packages\/react-dom\/src\/events\/DOMModernPluginEventSystem.js\nindex 095e1b6ce8..03ecd50ade 100644\n--- a\/packages\/react-dom\/src\/events\/DOMModernPluginEventSystem.js\n+++ b\/packages\/react-dom\/src\/events\/DOMModernPluginEventSystem.js\n@@ -417,13 +417,13 @@ export function dispatchEventForPluginEventSystem(\n \/\/ sub-tree for that root and make that our ancestor instance.\n let node = targetInst;\n \n- while (true) {\n+ mainLoop: while (true) {\n if (node === null) {\n return;\n }\n const nodeTag = node.tag;\n if (nodeTag === HostRoot || nodeTag === HostPortal) {\n- const container = node.stateNode.containerInfo;\n+ let container = node.stateNode.containerInfo;\n if (isMatchingRootContainer(container, targetContainerNode)) {\n break;\n }\n@@ -449,18 +449,23 @@ export function dispatchEventForPluginEventSystem(\n grandNode = grandNode.return;\n }\n }\n- const parentSubtreeInst = getClosestInstanceFromNode(container);\n- if (parentSubtreeInst === null) {\n- return;\n- }\n- const parentTag = parentSubtreeInst.tag;\n- \/\/ getClosestInstanceFromNode can return a HostRoot or SuspenseComponent.\n- \/\/ So we need to ensure we only set the ancestor to a HostComponent or HostText.\n- if (parentTag === HostComponent || parentTag === HostText) {\n- ancestorInst = parentSubtreeInst;\n+ \/\/ Now we need to find it's corresponding host fiber in the other\n+ \/\/ tree. To do this we can use getClosestInstanceFromNode, but we\n+ \/\/ need to validate that the fiber is a host instance, otherwise\n+ \/\/ we need to traverse up through the DOM till we find the correct\n+ \/\/ node that is from the other tree.\n+ while (container !== null) {\n+ const parentNode = getClosestInstanceFromNode(container);\n+ if (parentNode === null) {\n+ return;\n+ }\n+ const parentTag = parentNode.tag;\n+ if (parentTag === HostComponent || parentTag === HostText) {\n+ node = ancestorInst = parentNode;\n+ continue mainLoop;\n+ }\n+ container = container.parentNode;\n }\n- node = parentSubtreeInst;\n- continue;\n }\n node = node.return;\n }\ndiff --git a\/packages\/react-dom\/src\/events\/__tests__\/DOMModernPluginEventSystem-test.internal.js b\/packages\/react-dom\/src\/events\/__tests__\/DOMModernPluginEventSystem-test.internal.js\nindex 538353fff4..a690b574e9 100644\n--- a\/packages\/react-dom\/src\/events\/__tests__\/DOMModernPluginEventSystem-test.internal.js\n+++ b\/packages\/react-dom\/src\/events\/__tests__\/DOMModernPluginEventSystem-test.internal.js\n@@ -224,6 +224,95 @@ describe('DOMModernPluginEventSystem', () => {\n expect(log[5]).toEqual(['bubble', buttonElement]);\n });\n \n+ it('handle propagation of click events between disjointed roots #2', () => {\n+ const buttonRef = React.createRef();\n+ const button2Ref = React.createRef();\n+ const divRef = React.createRef();\n+ const spanRef = React.createRef();\n+ const log = [];\n+ const onClick = jest.fn(e => log.push(['bubble', e.currentTarget]));\n+ const onClickCapture = jest.fn(e =>\n+ log.push(['capture', e.currentTarget]),\n+ );\n+\n+ function Child() {\n+ return (\n+ \n+ Click me!\n+ <\/div>\n+ );\n+ }\n+\n+ function Parent() {\n+ return (\n+ \n+ );\n+ }\n+\n+ function GrandParent() {\n+ return (\n+ \n+ \n+ <\/button>\n+ );\n+ }\n+\n+ \/\/ We make a wrapper with an inner container that we\n+ \/\/ render to. So it looks like

<\/span><\/div>\n+ \/\/ We then render to all three:\n+ \/\/ - container\n+ \/\/ - parentContainer\n+ \/\/ - childContainer\n+\n+ const parentContainer = document.createElement('div');\n+ const childContainer = document.createElement('div');\n+\n+ ReactDOM.render(, container);\n+ ReactDOM.render(, parentContainer);\n+ ReactDOM.render(, childContainer);\n+\n+ parentContainer.appendChild(childContainer);\n+ spanRef.current.appendChild(parentContainer);\n+\n+ \/\/ Inside \n+ const buttonElement = buttonRef.current;\n+ dispatchClickEvent(buttonElement);\n+ expect(onClick).toHaveBeenCalledTimes(1);\n+ expect(onClickCapture).toHaveBeenCalledTimes(1);\n+ expect(log[0]).toEqual(['capture', buttonElement]);\n+ expect(log[1]).toEqual(['bubble', buttonElement]);\n+\n+ \/\/ Inside \n+ const divElement = divRef.current;\n+ dispatchClickEvent(divElement);\n+ expect(onClick).toHaveBeenCalledTimes(3);\n+ expect(onClickCapture).toHaveBeenCalledTimes(3);\n+ expect(log[2]).toEqual(['capture', divElement]);\n+ expect(log[3]).toEqual(['bubble', divElement]);\n+ expect(log[4]).toEqual(['capture', buttonElement]);\n+ expect(log[5]).toEqual(['bubble', buttonElement]);\n+\n+ \/\/ Inside \n+ const buttonElement2 = button2Ref.current;\n+ dispatchClickEvent(buttonElement2);\n+ expect(onClick).toHaveBeenCalledTimes(5);\n+ expect(onClickCapture).toHaveBeenCalledTimes(5);\n+ expect(log[6]).toEqual(['capture', buttonElement2]);\n+ expect(log[7]).toEqual(['bubble', buttonElement2]);\n+ expect(log[8]).toEqual(['capture', buttonElement]);\n+ expect(log[9]).toEqual(['bubble', buttonElement]);\n+ });\n+\n it('handle propagation of click events between disjointed comment roots', () => {\n const buttonRef = React.createRef();\n const divRef = React.createRef();\n"}

dillon1000/react

Files changed