Address feedback

facebook · Mar 19, 2020 · ea3ae21 · ea3ae21
1 parent 66f7c8d
commit ea3ae21
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 11 deletions.
diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js
@@ -67,6 +67,7 @@ import {
   TOP_RATE_CHANGE,
   TOP_PROGRESS,
   TOP_PLAYING,
+  TOP_CLICK,
 } from './DOMTopLevelEventTypes';
 import {
   getClosestInstanceFromNode,
@@ -229,7 +230,7 @@ function willDeferLaterForLegacyFBSupport(
   topLevelType: DOMTopLevelEventType,
   targetContainer: EventTarget,
 ): boolean {
-  if ((topLevelType: any) !== 'click') {
+  if (topLevelType !== TOP_CLICK) {
     return false;
   }
   // We defer all click events with legacy FB support mode on.
@@ -290,7 +291,12 @@ export function dispatchEventForPluginEventSystem(
     // time event listener so we can defer the event.
     if (
       enableLegacyFBSupport &&
+      // We do not want to defer if the event system has already been
+      // set to LEGACY_FB_SUPPORT. LEGACY_FB_SUPPORT only gets set when
+      // we call willDeferLaterForLegacyFBSupport, thus not bailing out
+      // will result in endless cycles like an infinite loop.
       (eventSystemFlags & LEGACY_FB_SUPPORT) === 0 &&
+      // We also don't want to defer during event replaying.
       (eventSystemFlags & IS_REPLAYED) === 0 &&
       willDeferLaterForLegacyFBSupport(topLevelType, targetContainer)
     ) {

diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js
@@ -133,7 +133,7 @@ export function addTrappedEventListener(
   targetContainer: EventTarget,
   topLevelType: DOMTopLevelEventType,
   capture: boolean,
-  legacyFBSupport?: boolean,
+  isDeferredListenerForLegacyFBSupport?: boolean,
   passive?: boolean,
   priority?: EventPriority,
 ): any => void {
@@ -161,7 +161,7 @@ export function addTrappedEventListener(
     passive = false;
   }
   const eventSystemFlags =
-    enableLegacyFBSupport && legacyFBSupport
+    enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport
       ? PLUGIN_EVENT_SYSTEM | LEGACY_FB_SUPPORT
       : PLUGIN_EVENT_SYSTEM;
 
@@ -182,8 +182,8 @@ export function addTrappedEventListener(
     targetContainer = document;
   }
 
-  const targetContainerToUse =
-    enableLegacyFBSupport && legacyFBSupport
+  targetContainer =
+    enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport
       ? (targetContainer: any).ownerDocument
       : targetContainer;
 
@@ -201,14 +201,14 @@ export function addTrappedEventListener(
   // browsers do not support this today, and given this is
   // to support legacy code patterns, it's likely they'll
   // need support for such browsers.
-  if (enableLegacyFBSupport && legacyFBSupport) {
+  if (enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport) {
     const originalListener = listener;
     listener = function(...p) {
       try {
         return originalListener.apply(this, p);
       } finally {
         removeEventListener(
-          targetContainerToUse,
+          targetContainer,
           rawEventName,
           unsubscribeListener,
           capture,
@@ -220,14 +220,14 @@ export function addTrappedEventListener(
     if (enableUseEventAPI && passive !== undefined) {
       // This is only used with passive is either true or false.
       unsubscribeListener = addEventCaptureListenerWithPassiveFlag(
-        targetContainerToUse,
+        targetContainer,
         rawEventName,
         listener,
         passive,
       );
     } else {
       unsubscribeListener = addEventCaptureListener(
-        targetContainerToUse,
+        targetContainer,
         rawEventName,
         listener,
       );
@@ -236,14 +236,14 @@ export function addTrappedEventListener(
     if (enableUseEventAPI && passive !== undefined) {
       // This is only used with passive is either true or false.
       unsubscribeListener = addEventBubbleListenerWithPassiveFlag(
-        targetContainerToUse,
+        targetContainer,
         rawEventName,
         listener,
         passive,
       );
     } else {
       unsubscribeListener = addEventBubbleListener(
-        targetContainerToUse,
+        targetContainer,
         rawEventName,
         listener,
       );