[react-events] Fix Scope listener issue (#16658)

facebook · Sep 4, 2019 · 9ff60ff · 9ff60ff
1 parent 7126a37
commit 9ff60ff
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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<any, any>,
     ReactEventResponderInstance<any, any>,
   >,
+  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<any, any>,
     ReactEventResponderInstance<any, any>,
   >,
+  rootContainerInstance: null | Container,
 ): void {
   let responder;
   let props;
@@ -127,15 +132,25 @@ 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;
     responderInstance.fiber = fiber;
   }
 }
 
-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
@@ -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(<Component />, 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 (
+          <div>
+            <TestScope listeners={listener}>
+              <div ref={ref} />
+            </TestScope>
+          </div>
+        );
+      };
+      ReactDOM.render(<Component />, container);
+
+      target = createEventTarget(ref.current);
       target.keydown({key: 'Q'});
       expect(onKeyDown).toHaveBeenCalledTimes(1);
       expect(onKeyDown).toHaveBeenCalledWith(