Flare: Fix listener upgrade bug (#18270)

facebook · Mar 10, 2020 · a3bf668 · a3bf668
1 parent 526c12f
commit a3bf668
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 32 deletions.
diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js
@@ -59,7 +59,7 @@ import {
 import {getListenerMapForElement} from '../events/DOMEventListenerMap';
 import {
   addResponderEventSystemEvent,
-  removeActiveResponderEventSystemEvent,
+  removeTrappedPassiveEventListener,
 } from '../events/ReactDOMEventListener.js';
 import {mediaEventTypes} from '../events/DOMTopLevelEventTypes';
 import {
@@ -1360,7 +1360,7 @@ export function listenToEventResponderEventTypes(
           const passiveKey = targetEventType + '_passive';
           const passiveListener = listenerMap.get(passiveKey);
           if (passiveListener != null) {
-            removeActiveResponderEventSystemEvent(
+            removeTrappedPassiveEventListener(
               document,
               targetEventType,
               passiveListener,

diff --git a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js
@@ -39,7 +39,7 @@ import {
   getRawEventName,
   mediaEventTypes,
 } from './DOMTopLevelEventTypes';
-import {trapEventForPluginEventSystem} from './ReactDOMEventListener';
+import {addTrappedEventListener} from './ReactDOMEventListener';
 
 /**
  * Summary of `DOMEventPluginSystem` event handling:
@@ -368,12 +368,12 @@ export function legacyTrapBubbledEvent(
   topLevelType: DOMTopLevelEventType,
   element: Document | Element,
 ): void {
-  trapEventForPluginEventSystem(element, topLevelType, false);
+  addTrappedEventListener(element, topLevelType, false);
 }
 
 export function legacyTrapCapturedEvent(
   topLevelType: DOMTopLevelEventType,
   element: Document | Element,
 ): void {
-  trapEventForPluginEventSystem(element, topLevelType, true);
+  addTrappedEventListener(element, topLevelType, true);
 }
diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js
@@ -21,7 +21,7 @@ import {plugins} from 'legacy-events/EventPluginRegistry';
 
 import {HostRoot, HostPortal} from 'shared/ReactWorkTags';
 
-import {trapEventForPluginEventSystem} from './ReactDOMEventListener';
+import {addTrappedEventListener} from './ReactDOMEventListener';
 import getEventTarget from './getEventTarget';
 import {getListenerMapForElement} from './DOMEventListenerMap';
 import {
@@ -149,11 +149,7 @@ export function listenToTopLevelEvent(
 ): void {
   if (!listenerMap.has(topLevelType)) {
     const isCapturePhase = capturePhaseEvents.has(topLevelType);
-    trapEventForPluginEventSystem(
-      rootContainerElement,
-      topLevelType,
-      isCapturePhase,
-    );
+    addTrappedEventListener(rootContainerElement, topLevelType, isCapturePhase);
     listenerMap.set(topLevelType, null);
   }
 }
@@ -196,7 +192,7 @@ function willDeferLaterForFBLegacyPrimer(nativeEvent: any): boolean {
     if (node.tagName === 'A' && validFBLegacyPrimerRels.has(node.rel)) {
       const legacyFBSupport = true;
       const isCapture = nativeEvent.eventPhase === 1;
-      trapEventForPluginEventSystem(
+      addTrappedEventListener(
         document,
         ((type: any): DOMTopLevelEventType),
         isCapture,

diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js
@@ -105,6 +105,7 @@ export function addResponderEventSystemEvent(
   } else {
     eventFlags |= IS_ACTIVE;
   }
+  let fbListener;
   // Check if interactive and wrap in discreteUpdates
   const listener = dispatchEvent.bind(
     null,
@@ -113,39 +114,27 @@ export function addResponderEventSystemEvent(
     document,
   );
   if (passiveBrowserEventsSupported) {
-    addEventCaptureListenerWithPassiveFlag(
+    fbListener = addEventCaptureListenerWithPassiveFlag(
       document,
       topLevelType,
       listener,
       passive,
     );
   } else {
-    addEventCaptureListener(document, topLevelType, listener);
-  }
-  return listener;
-}
-
-export function removeActiveResponderEventSystemEvent(
-  document: Document,
-  topLevelType: string,
-  listener: any => void,
-) {
-  if (passiveBrowserEventsSupported) {
-    document.removeEventListener(topLevelType, listener, {
-      capture: true,
-      passive: false,
-    });
-  } else {
-    document.removeEventListener(topLevelType, listener, true);
+    fbListener = addEventCaptureListener(document, topLevelType, listener);
   }
+  // If we have an fbListener, then use that.
+  // We'll only have one if we use the forked
+  // EventListener-www module in FB builds.
+  return fbListener || listener;
 }
 
-export function trapEventForPluginEventSystem(
+export function addTrappedEventListener(
   container: Document | Element,
   topLevelType: DOMTopLevelEventType,
   capture: boolean,
   legacyFBSupport?: boolean,
-): void {
+): any => void {
   let listener;
   let listenerWrapper;
   switch (getEventPriorityForPluginSystem(topLevelType)) {
@@ -203,6 +192,29 @@ export function trapEventForPluginEventSystem(
   } else {
     fbListener = addEventBubbleListener(container, rawEventName, listener);
   }
+  // If we have an fbListener, then use that.
+  // We'll only have one if we use the forked
+  // EventListener-www module in FB builds.
+  return fbListener || listener;
+}
+
+export function removeTrappedPassiveEventListener(
+  document: Document,
+  topLevelType: string,
+  listener: any => void,
+) {
+  if (listener.remove != null) {
+    listener.remove();
+  } else {
+    if (passiveBrowserEventsSupported) {
+      document.removeEventListener(topLevelType, listener, {
+        capture: true,
+        passive: true,
+      });
+    } else {
+      document.removeEventListener(topLevelType, listener, true);
+    }
+  }
 }
 
 function dispatchDiscreteEvent(

diff --git a/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js
@@ -948,4 +948,39 @@ describe('DOMEventResponderSystem', () => {
     document.body.removeChild(domNode);
     expect(onEvent).toBeCalled();
   });
+
+  it('event upgrading should work correctly', () => {
+    let eventResponderFiredCount = 0;
+    const buttonRef = React.createRef();
+
+    const TestResponder = createEventResponder({
+      targetEventTypes: ['click'],
+      onEvent: (event, context, props, state) => {
+        eventResponderFiredCount++;
+        if (!state.addedRootEventTypes) {
+          context.addRootEventTypes(['click_active']);
+        }
+        state.addedRootEventTypes = true;
+      },
+    });
+
+    function Test() {
+      const listener = React.DEPRECATED_useResponder(TestResponder, {});
+
+      return (
+        <button ref={buttonRef} DEPRECATED_flareListeners={listener}>
+          Click me!
+        </button>
+      );
+    }
+
+    ReactDOM.render(<Test />, container);
+    expect(container.innerHTML).toBe('<button>Click me!</button>');
+
+    let buttonElement = buttonRef.current;
+    dispatchClickEvent(buttonElement);
+    expect(eventResponderFiredCount).toBe(1);
+    dispatchClickEvent(buttonElement);
+    expect(eventResponderFiredCount).toBe(2);
+  });
 });