Don't use EventListener Fork in Modern WWW Builds (#18333)

* Move unsubscribe fork to EventListener That way we can statically compile out more of these indirections. * Don't use the EventListener fork for Modern WWW builds
facebook · Mar 18, 2020 · 94505b9 · 94505b9
1 parent aae83a4
commit 94505b9
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 44 deletions.
diff --git a/packages/react-dom/src/events/EventListener.js b/packages/react-dom/src/events/EventListener.js
@@ -11,37 +11,50 @@ export function addEventBubbleListener(
   target: EventTarget,
   eventType: string,
   listener: Function,
-): void {
+): Function {
   target.addEventListener(eventType, listener, false);
+  return listener;
 }
 
 export function addEventCaptureListener(
   target: EventTarget,
   eventType: string,
   listener: Function,
-): void {
+): Function {
   target.addEventListener(eventType, listener, true);
+  return listener;
 }
 
 export function addEventCaptureListenerWithPassiveFlag(
   target: EventTarget,
   eventType: string,
   listener: Function,
   passive: boolean,
-): void {
+): Function {
   target.addEventListener(eventType, listener, {
     capture: true,
     passive,
   });
+  return listener;
 }
 
 export function addEventBubbleListenerWithPassiveFlag(
   target: EventTarget,
   eventType: string,
   listener: Function,
   passive: boolean,
-): void {
+): Function {
   target.addEventListener(eventType, listener, {
     passive,
   });
+  return listener;
+}
+
+export function removeEventListener(
+  target: EventTarget,
+  eventType: string,
+  listener: Function,
+  capture: boolean,
+): void {
+  target.removeEventListener(eventType, listener, capture);
 }
diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js
@@ -49,6 +49,7 @@ import {
   addEventCaptureListener,
   addEventCaptureListenerWithPassiveFlag,
   addEventBubbleListenerWithPassiveFlag,
+  removeEventListener,
 } from './EventListener';
 import getEventTarget from './getEventTarget';
 import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree';
@@ -108,7 +109,6 @@ export function addResponderEventSystemEvent(
   } else {
     eventFlags |= IS_ACTIVE;
   }
-  let fbListener;
   // Check if interactive and wrap in discreteUpdates
   const listener = dispatchEvent.bind(
     null,
@@ -117,19 +117,15 @@ export function addResponderEventSystemEvent(
     document,
   );
   if (passiveBrowserEventsSupported) {
-    fbListener = addEventCaptureListenerWithPassiveFlag(
+    return addEventCaptureListenerWithPassiveFlag(
       document,
       topLevelType,
       listener,
       passive,
     );
   } else {
-    fbListener = addEventCaptureListener(document, topLevelType, listener);
+    return 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 addTrappedEventListener(
@@ -184,8 +180,8 @@ export function addTrappedEventListener(
   const validTargetContainer = ((targetContainer: any): EventTarget);
 
   const rawEventName = getRawEventName(topLevelType);
-  let fbListener;
 
+  let unsubscribeListener;
   // When legacyFBSupport is enabled, it's for when we
   // want to add a one time event listener to a container.
   // This should only be used with enableLegacyFBPrimerSupport
@@ -203,28 +199,26 @@ export function addTrappedEventListener(
       try {
         return originalListener.apply(this, p);
       } finally {
-        if (fbListener) {
-          fbListener.remove();
-        } else {
-          validTargetContainer.removeEventListener(
-            ((rawEventName: any): string),
-            (listener: any),
-          );
-        }
+        removeEventListener(
+          validTargetContainer,
+          rawEventName,
+          unsubscribeListener,
+          capture,
+        );
       }
     };
   }
   if (capture) {
     if (enableUseEventAPI && passive !== undefined) {
       // This is only used with passive is either true or false.
-      fbListener = addEventCaptureListenerWithPassiveFlag(
+      unsubscribeListener = addEventCaptureListenerWithPassiveFlag(
         validTargetContainer,
         rawEventName,
         listener,
         passive,
       );
     } else {
-      fbListener = addEventCaptureListener(
+      unsubscribeListener = addEventCaptureListener(
         validTargetContainer,
         rawEventName,
         listener,
@@ -233,24 +227,21 @@ export function addTrappedEventListener(
   } else {
     if (enableUseEventAPI && passive !== undefined) {
       // This is only used with passive is either true or false.
-      fbListener = addEventBubbleListenerWithPassiveFlag(
+      unsubscribeListener = addEventBubbleListenerWithPassiveFlag(
         validTargetContainer,
         rawEventName,
         listener,
         passive,
       );
     } else {
-      fbListener = addEventBubbleListener(
+      unsubscribeListener = addEventBubbleListener(
         validTargetContainer,
         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;
+  return unsubscribeListener;
 }
 
 export function removeTrappedEventListener(
@@ -259,20 +250,9 @@ export function removeTrappedEventListener(
   capture: boolean,
   listener: any => void,
   passive: void | boolean,
-) {
-  if (listener.remove != null) {
-    listener.remove();
-  } else {
-    const rawEventName = getRawEventName(topLevelType);
-    if (passiveBrowserEventsSupported) {
-      targetContainer.removeEventListener(rawEventName, listener, {
-        capture,
-        passive,
-      });
-    } else {
-      targetContainer.removeEventListener(rawEventName, listener, capture);
-    }
-  }
+): void {
+  const rawEventName = getRawEventName(topLevelType);
+  removeEventListener(targetContainer, rawEventName, listener, capture);
 }
 
 function dispatchDiscreteEvent(

diff --git a/packages/react-dom/src/events/forks/EventListener-www.js b/packages/react-dom/src/events/forks/EventListener-www.js
@@ -56,6 +56,15 @@ export function addEventBubbleListenerWithPassiveFlag(
   );
 }
 
+export function removeEventListener(
+  target: EventTarget,
+  eventType: string,
+  listener: Function,
+  capture: boolean,
+) {
+  listener.remove();
+}
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js
@@ -19,6 +19,15 @@ const {
 } = bundleTypes;
 const {RENDERER, RECONCILER} = moduleTypes;
 
+const RELEASE_CHANNEL = process.env.RELEASE_CHANNEL;
+
+// Default to building in experimental mode. If the release channel is set via
+// an environment variable, then check if it's "experimental".
+const __EXPERIMENTAL__ =
+  typeof RELEASE_CHANNEL === 'string'
+    ? RELEASE_CHANNEL === 'experimental'
+    : true;
+
 // If you need to replace a file with another file for a specific environment,
 // add it to this list with the logic for choosing the right replacement.
 const forks = Object.freeze({
@@ -442,8 +451,13 @@ const forks = Object.freeze({
       case FB_WWW_DEV:
       case FB_WWW_PROD:
       case FB_WWW_PROFILING:
-        // Use the www fork which is integrated with TimeSlice profiling.
-        return 'react-dom/src/events/forks/EventListener-www.js';
+        if (__EXPERIMENTAL__) {
+          // In modern builds we don't use the indirection. We just use raw DOM.
+          return null;
+        } else {
+          // Use the www fork which is integrated with TimeSlice profiling.
+          return 'react-dom/src/events/forks/EventListener-www.js';
+        }
       default:
         return null;
     }