ReactDOM.useEvent: revert and add guard for null stateNode

Add accumulateUseEventListeners flag Fix Fxi Add support for HostText being target Fix
facebook · Mar 31, 2020 · c9c5e44 · c9c5e44
1 parent 515957d
commit c9c5e44
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 44 deletions.
diff --git a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js
@@ -1317,6 +1317,34 @@ describe('DOMModernPluginEventSystem', () => {
             expect(clickEvent).toBeCalledTimes(0);
           });
 
+          it('should handle the target being a text node', () => {
+            const clickEvent = jest.fn();
+            const buttonRef = React.createRef();
+
+            function Test({off}) {
+              const click = ReactDOM.unstable_useEvent('click');
+
+              React.useEffect(() => {
+                click.setListener(buttonRef.current, clickEvent);
+              });
+
+              React.useEffect(() => {
+                if (off) {
+                  click.setListener(buttonRef.current, null);
+                }
+              }, [off]);
+
+              return <button ref={buttonRef}>Click me!</button>;
+            }
+
+            ReactDOM.render(<Test off={false} />, container);
+            Scheduler.unstable_flushAll();
+
+            let textNode = buttonRef.current.firstChild;
+            dispatchClickEvent(textNode);
+            expect(clickEvent).toBeCalledTimes(1);
+          });
+
           it('handle propagation of click events', () => {
             const buttonRef = React.createRef();
             const divRef = React.createRef();

diff --git a/packages/react-dom/src/events/accumulateTwoPhaseListeners.js b/packages/react-dom/src/events/accumulateTwoPhaseListeners.js
@@ -14,8 +14,13 @@ import type {ReactSyntheticEvent} from 'legacy-events/ReactSyntheticEventType';
 import {
   HostComponent,
   ScopeComponent,
+  HostText,
 } from 'react-reconciler/src/ReactWorkTags';
-import {enableUseEventAPI, enableScopeAPI} from 'shared/ReactFeatureFlags';
+import {
+  enableUseEventAPI,
+  enableScopeAPI,
+  enableModernEventSystem,
+} from 'shared/ReactFeatureFlags';
 
 import getListener from 'legacy-events/getListener';
 import {getListenersFromTarget} from '../client/ReactDOMComponentTree';
@@ -82,60 +87,78 @@ export default function accumulateTwoPhaseListeners(
     // usual two phase accumulation using the React fiber tree to pick up
     // all relevant useEvent and on* prop events.
     let node = event._targetInst;
-    let lastHostComponent;
+    let lastHostComponent = null;
 
     // Accumulate all instances and listeners via the target -> root path.
     while (node !== null) {
-      // Handle listeners that are on HostComponents (i.e. <div>)
-      if (node.tag === HostComponent) {
-        lastHostComponent = node.stateNode;
-        // For useEvent listenrs
-        if (enableUseEventAPI && accumulateUseEventListeners) {
-          // useEvent event listeners
-          const instance = node.stateNode;
-          const targetType = event.type;
-          const listeners = getListenersFromTarget(instance);
+      const {stateNode: instance, tag} = node;
+      // If there is no instance, then the HostComponent/HostText is not
+      // mounted.
+      if (instance !== null) {
+        // Handle listeners that are on HostComponents (i.e. <div>)
+        if (tag === HostComponent) {
+          lastHostComponent = instance;
+          // For useEvent listenrs
+          if (
+            enableModernEventSystem &&
+            enableUseEventAPI &&
+            accumulateUseEventListeners
+          ) {
+            // useEvent event listeners
+            const targetType = event.type;
+            const listeners = getListenersFromTarget(instance);
 
-          if (listeners !== null) {
-            const listenersArr = Array.from(listeners);
-            for (let i = 0; i < listenersArr.length; i++) {
-              const listener = listenersArr[i];
-              const {
-                callback,
-                event: {capture, type},
-              } = listener;
-              if (type === targetType) {
-                if (capture === true) {
-                  dispatchListeners.unshift(callback);
-                  dispatchInstances.unshift(node);
-                } else {
-                  dispatchListeners.push(callback);
-                  dispatchInstances.push(node);
+            if (listeners !== null) {
+              const listenersArr = Array.from(listeners);
+              for (let i = 0; i < listenersArr.length; i++) {
+                const listener = listenersArr[i];
+                const {
+                  callback,
+                  event: {capture, type},
+                } = listener;
+                if (type === targetType) {
+                  if (capture === true) {
+                    dispatchListeners.unshift(callback);
+                    dispatchInstances.unshift(node);
+                  } else {
+                    dispatchListeners.push(callback);
+                    dispatchInstances.push(node);
+                  }
                 }
               }
             }
           }
-        }
-        // Standard React on* listeners, i.e. onClick prop
-        if (captured !== null) {
-          const captureListener = getListener(node, captured);
-          if (captureListener != null) {
-            // Capture listeners/instances should go at the start, so we
-            // unshift them to the start of the array.
-            dispatchListeners.unshift(captureListener);
-            dispatchInstances.unshift(node);
+          // Standard React on* listeners, i.e. onClick prop
+          if (captured !== null) {
+            const captureListener = getListener(node, captured);
+            if (captureListener != null) {
+              // Capture listeners/instances should go at the start, so we
+              // unshift them to the start of the array.
+              dispatchListeners.unshift(captureListener);
+              dispatchInstances.unshift(node);
+            }
           }
-        }
-        if (bubbled !== null) {
-          const bubbleListener = getListener(node, bubbled);
-          if (bubbleListener != null) {
-            // Bubble listeners/instances should go at the end, so we
-            // push them to the end of the array.
-            dispatchListeners.push(bubbleListener);
-            dispatchInstances.push(node);
+          if (bubbled !== null) {
+            const bubbleListener = getListener(node, bubbled);
+            if (bubbleListener != null) {
+              // Bubble listeners/instances should go at the end, so we
+              // push them to the end of the array.
+              dispatchListeners.push(bubbleListener);
+              dispatchInstances.push(node);
+            }
           }
+        } else if (tag === HostText) {
+          lastHostComponent = instance;
         }
-      } else if (enableScopeAPI && node.tag === ScopeComponent) {
+      }
+      if (
+        enableModernEventSystem &&
+        enableUseEventAPI &&
+        enableScopeAPI &&
+        accumulateUseEventListeners &&
+        tag === ScopeComponent &&
+        lastHostComponent !== null
+      ) {
         const reactScope = node.stateNode.methods;
         const eventTypeMap = reactScopeListenerStore.get(reactScope);
         if (eventTypeMap !== undefined) {