From 9ff60ff16b289a9efce46e21fe223aec1638b5d6 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 4 Sep 2019 18:57:39 +0100 Subject: [PATCH] [react-events] Fix Scope listener issue (#16658) --- .../src/ReactFiberCommitWork.js | 4 +- .../src/ReactFiberCompleteWork.js | 19 +++++- .../react-reconciler/src/ReactFiberEvents.js | 61 +++++++++++++------ .../src/__tests__/ReactScope-test.internal.js | 28 ++++++++- 4 files changed, 87 insertions(+), 25 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 44cb2e59d6a92..7ac9433cfcb97 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1343,7 +1343,7 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { const prevListeners = oldProps.listeners; const nextListeners = newProps.listeners; if (prevListeners !== nextListeners) { - updateEventListeners(nextListeners, finishedWork); + updateEventListeners(nextListeners, finishedWork, null); } } } @@ -1400,7 +1400,7 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { const prevListeners = oldProps.listeners; const nextListeners = newProps.listeners; if (prevListeners !== nextListeners) { - updateEventListeners(nextListeners, finishedWork); + updateEventListeners(nextListeners, finishedWork, null); } } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index dc8558ef54176..675db8858f224 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -724,7 +724,11 @@ function completeWork( if (enableFlareAPI) { const listeners = newProps.listeners; if (listeners != null) { - updateEventListeners(listeners, workInProgress); + updateEventListeners( + listeners, + workInProgress, + rootContainerInstance, + ); } } } else { @@ -744,7 +748,11 @@ function completeWork( if (enableFlareAPI) { const listeners = newProps.listeners; if (listeners != null) { - updateEventListeners(listeners, workInProgress); + updateEventListeners( + listeners, + workInProgress, + rootContainerInstance, + ); } } @@ -1233,7 +1241,12 @@ function completeWork( if (enableFlareAPI) { const listeners = newProps.listeners; if (listeners != null) { - updateEventListeners(listeners, workInProgress); + const rootContainerInstance = getRootHostContainer(); + updateEventListeners( + listeners, + workInProgress, + rootContainerInstance, + ); } } if (workInProgress.ref !== null) { diff --git a/packages/react-reconciler/src/ReactFiberEvents.js b/packages/react-reconciler/src/ReactFiberEvents.js index 90fe39a8d3674..af3b23b11a9b0 100644 --- a/packages/react-reconciler/src/ReactFiberEvents.js +++ b/packages/react-reconciler/src/ReactFiberEvents.js @@ -8,7 +8,7 @@ */ import type {Fiber} from './ReactFiber'; -import type {Instance} from './ReactFiberHostConfig'; +import type {Container, Instance} from './ReactFiberHostConfig'; import type { ReactEventResponder, ReactEventResponderInstance, @@ -53,6 +53,7 @@ function mountEventResponder( ReactEventResponder, ReactEventResponderInstance, >, + rootContainerInstance: null | Container, ) { let responderState = emptyObject; const getInitialState = responder.getInitialState; @@ -65,25 +66,28 @@ function mountEventResponder( responderState, fiber, ); - let instance = null; - let node = fiber; - while (node !== null) { - const tag = node.tag; - if (tag === HostComponent) { - instance = node.stateNode; - break; - } else if (tag === HostRoot) { - instance = node.stateNode.containerInfo; - break; + + if (!rootContainerInstance) { + let node = fiber; + while (node !== null) { + const tag = node.tag; + if (tag === HostComponent) { + rootContainerInstance = node.stateNode; + break; + } else if (tag === HostRoot) { + rootContainerInstance = node.stateNode.containerInfo; + break; + } + node = node.return; } - node = node.return; } + mountResponderInstance( responder, responderInstance, responderProps, responderState, - ((instance: any): Instance), + ((rootContainerInstance: any): Instance), ); respondersMap.set(responder, responderInstance); } @@ -96,6 +100,7 @@ function updateEventListener( ReactEventResponder, ReactEventResponderInstance, >, + rootContainerInstance: null | Container, ): void { let responder; let props; @@ -127,7 +132,13 @@ function updateEventListener( if (responderInstance === undefined) { // Mount (happens in either complete or commit phase) - mountEventResponder(responder, listenerProps, fiber, respondersMap); + mountEventResponder( + responder, + listenerProps, + fiber, + respondersMap, + rootContainerInstance, + ); } else { // Update (happens during commit phase only) responderInstance.props = listenerProps; @@ -135,7 +146,11 @@ function updateEventListener( } } -export function updateEventListeners(listeners: any, fiber: Fiber): void { +export function updateEventListeners( + listeners: any, + fiber: Fiber, + rootContainerInstance: null | Container, +): void { const visistedResponders = new Set(); let dependencies = fiber.dependencies; if (listeners != null) { @@ -153,10 +168,22 @@ export function updateEventListeners(listeners: any, fiber: Fiber): void { if (isArray(listeners)) { for (let i = 0, length = listeners.length; i < length; i++) { const listener = listeners[i]; - updateEventListener(listener, fiber, visistedResponders, respondersMap); + updateEventListener( + listener, + fiber, + visistedResponders, + respondersMap, + rootContainerInstance, + ); } } else { - updateEventListener(listeners, fiber, visistedResponders, respondersMap); + updateEventListener( + listeners, + fiber, + visistedResponders, + respondersMap, + rootContainerInstance, + ); } } if (dependencies !== null) { diff --git a/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js b/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js index 77bde7763e95a..9ceca7f77268e 100644 --- a/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js @@ -199,11 +199,11 @@ describe('ReactScope', () => { }); it('event responders can be attached to scopes', () => { + let onKeyDown = jest.fn(); const TestScope = React.unstable_createScope((type, props) => true); - const onKeyDown = jest.fn(); const ref = React.createRef(); const useKeyboard = require('react-events/keyboard').useKeyboard; - const Component = () => { + let Component = () => { const listener = useKeyboard({ onKeyDown, }); @@ -215,7 +215,29 @@ describe('ReactScope', () => { }; ReactDOM.render(, container); - const target = createEventTarget(ref.current); + let target = createEventTarget(ref.current); + target.keydown({key: 'Q'}); + expect(onKeyDown).toHaveBeenCalledTimes(1); + expect(onKeyDown).toHaveBeenCalledWith( + expect.objectContaining({key: 'Q', type: 'keydown'}), + ); + + onKeyDown = jest.fn(); + Component = () => { + const listener = useKeyboard({ + onKeyDown, + }); + return ( +
+ +
+ +
+ ); + }; + ReactDOM.render(, container); + + target = createEventTarget(ref.current); target.keydown({key: 'Q'}); expect(onKeyDown).toHaveBeenCalledTimes(1); expect(onKeyDown).toHaveBeenCalledWith(