Repository: facebook/react
Pull request: Decouple dispatching from attemptToDispatchEvent

**Purely refactoring, no functional changes.**

We need to split up dispatching from finding the blocked instance, so that we can apply different logic in each case (e.g. replay instead of dispatching through the event system). In #22680 we tried to do this via a special return type, but it's really noisy and I find it difficult to verify the existing behavior is intact. This is an alternative proposal, where we just use a few fields as makeshift "return values" to avoid allocating. I think I've seen this somewhere in the code. (I mean, we could also just use a tuple but idk if this is controversial for some reason.)

If we land this I think we can make #22680 simpler and more self-contained.

Changed files:
filename: packages/react-dom/src/events/ReactDOMEventListener.js
patch: @@ -180,15 +180,20 @@ export function dispatchEvent(
     return;
   }
 
-  let blockedOn = attemptToDispatchEvent(
+  let blockedOn = findInstanceBlockingEvent(
     domEventName,
     eventSystemFlags,
     targetContainer,
     nativeEvent,
   );
-
   if (blockedOn === null) {
-    // We successfully dispatched this event.
+    dispatchEventForPluginEventSystem(
+      domEventName,
+      eventSystemFlags,
+      nativeEvent,
+      return_targetInst,
+      targetContainer,
+    );
     if (allowReplay) {
       clearIfContinuousEvent(domEventName, nativeEvent);
     }
@@ -236,12 +241,21 @@ export function dispatchEvent(
       if (fiber !== null) {
         attemptSynchronousHydration(fiber);
       }
-      const nextBlockedOn = attemptToDispatchEvent(
+      const nextBlockedOn = findInstanceBlockingEvent(
         domEventName,
         eventSystemFlags,
         targetContainer,
         nativeEvent,
       );
+      if (nextBlockedOn === null) {
+        dispatchEventForPluginEventSystem(
+          domEventName,
+          eventSystemFlags,
+          nativeEvent,
+          return_targetInst,
+          targetContainer,
+        );
+      }
       if (nextBlockedOn === blockedOn) {
         break;
       }
@@ -264,15 +278,20 @@ export function dispatchEvent(
   );
 }
 
-// Attempt dispatching an event. Returns a SuspenseInstance or Container if it's blocked.
-export function attemptToDispatchEvent(
+export let return_targetInst = null;
+
+// Returns a SuspenseInstance or Container if it's blocked.
+// The return_targetInst field above is conceptually part of the return value.
+export function findInstanceBlockingEvent(
   domEventName: DOMEventName,
   eventSystemFlags: EventSystemFlags,
   targetContainer: EventTarget,
   nativeEvent: AnyNativeEvent,
 ): null | Container | SuspenseInstance {
   // TODO: Warn if _enabled is false.
 
+  return_targetInst = null;
+
   const nativeEventTarget = getEventTarget(nativeEvent);
   let targetInst = getClosestInstanceFromNode(nativeEventTarget);
 
@@ -313,13 +332,7 @@ export function attemptToDispatchEvent(
       }
     }
   }
-  dispatchEventForPluginEventSystem(
-    domEventName,
-    eventSystemFlags,
-    nativeEvent,
-    targetInst,
-    targetContainer,
-  );
+  return_targetInst = targetInst;
   // We're not blocked on anything.
   return null;
 }

filename: packages/react-dom/src/events/ReactDOMEventReplaying.js
patch: @@ -27,7 +27,11 @@ import {
   getContainerFromFiber,
   getSuspenseInstanceFromFiber,
 } from 'react-reconciler/src/ReactFiberTreeReflection';
-import {attemptToDispatchEvent} from './ReactDOMEventListener';
+import {
+  findInstanceBlockingEvent,
+  return_targetInst,
+} from './ReactDOMEventListener';
+import {dispatchEventForPluginEventSystem} from './DOMPluginEventSystem';
 import {
   getInstanceFromNode,
   getClosestInstanceFromNode,
@@ -389,7 +393,7 @@ export function queueIfContinuousEvent(
 function attemptExplicitHydrationTarget(
   queuedTarget: QueuedHydrationTarget,
 ): void {
-  // TODO: This function shares a lot of logic with attemptToDispatchEvent.
+  // TODO: This function shares a lot of logic with findInstanceBlockingEvent.
   // Try to unify them. It's a bit tricky since it would require two return
   // values.
   const targetInst = getClosestInstanceFromNode(queuedTarget.target);
@@ -462,13 +466,21 @@ function attemptReplayContinuousQueuedEvent(
   const targetContainers = queuedEvent.targetContainers;
   while (targetContainers.length > 0) {
     const targetContainer = targetContainers[0];
-    const nextBlockedOn = attemptToDispatchEvent(
+    const nextBlockedOn = findInstanceBlockingEvent(
       queuedEvent.domEventName,
       queuedEvent.eventSystemFlags,
       targetContainer,
       queuedEvent.nativeEvent,
     );
-    if (nextBlockedOn !== null) {
+    if (nextBlockedOn === null) {
+      dispatchEventForPluginEventSystem(
+        queuedEvent.domEventName,
+        queuedEvent.eventSystemFlags,
+        queuedEvent.nativeEvent,
+        return_targetInst,
+        targetContainer,
+      );
+    } else {
       // We're still blocked. Try again later.
       const fiber = getInstanceFromNode(nextBlockedOn);
       if (fiber !== null) {
@@ -512,13 +524,21 @@ function replayUnblockedEvents() {
       const targetContainers = nextDiscreteEvent.targetContainers;
       while (targetContainers.length > 0) {
         const targetContainer = targetContainers[0];
-        const nextBlockedOn = attemptToDispatchEvent(
+        const nextBlockedOn = findInstanceBlockingEvent(
           nextDiscreteEvent.domEventName,
           nextDiscreteEvent.eventSystemFlags,
           targetContainer,
           nextDiscreteEvent.nativeEvent,
         );
-        if (nextBlockedOn !== null) {
+        if (nextBlockedOn === null) {
+          dispatchEventForPluginEventSystem(
+            nextDiscreteEvent.domEventName,
+            nextDiscreteEvent.eventSystemFlags,
+            nextDiscreteEvent.nativeEvent,
+            return_targetInst,
+            targetContainer,
+          );
+        } else {
           // We're still blocked. Try again later.
           nextDiscreteEvent.blockedOn = nextBlockedOn;
           break;