React Event System: Refactor ElementListenerMap for upgrading

packages/react-dom/src/client/ReactDOMComponent.js

@@ -1358,22 +1358,27 @@ export function listenToEventResponderEventTypes(
           // existing passive event listener before we add the
           // active event listener.
           const passiveKey = targetEventType + '_passive';
-          const passiveListener = listenerMap.get(passiveKey);
-          if (passiveListener != null) {
+          const passiveItem = listenerMap.get(passiveKey);
+          if (passiveItem !== undefined) {
             removeTrappedEventListener(
               document,
-              targetEventType,
-              passiveListener,
+              (targetEventType: any),
+              true,
+              passiveItem.listener,
               true,
             );
+            listenerMap.delete(passiveKey);
           }
         }
         const eventListener = addResponderEventSystemEvent(
           document,
           targetEventType,
           isPassive,
         );
-        listenerMap.set(eventKey, eventListener);
+        listenerMap.set(eventKey, {
+          passive: isPassive,
+          listener: eventListener,
+        });
       }
     }
   }

packages/react-dom/src/events/DOMEventListenerMap.js

@@ -16,13 +16,18 @@ const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
 const elementListenerMap:
   // $FlowFixMe Work around Flow bug
   | WeakMap
-  | Map<EventTarget, Map<DOMTopLevelEventType | string, null | (any => void)>> = new PossiblyWeakMap();
+  | Map<EventTarget, ElementListenerMap> = new PossiblyWeakMap();
 
 export type ElementListenerMap = Map<
   DOMTopLevelEventType | string,
-  null | (any => void),
+  ElementListenerMapEntry,
 >;
 
+export type ElementListenerMapEntry = {
+  passive: void | boolean,
+  listener: any => void,
+};
+
 export function getListenerMapForElement(
   target: EventTarget,
 ): ElementListenerMap {

packages/react-dom/src/events/DOMLegacyEventPluginSystem.js

@@ -9,6 +9,7 @@
 
 import type {AnyNativeEvent} from 'legacy-events/PluginModuleType';
 import type {DOMTopLevelEventType} from 'legacy-events/TopLevelEventTypes';
+import type {ElementListenerMap} from '../events/DOMEventListenerMap';
 import type {EventSystemFlags} from 'legacy-events/EventSystemFlags';
 import type {Fiber} from 'react-reconciler/src/ReactFiber';
 import type {PluginModule} from 'legacy-events/PluginModuleType';
@@ -323,57 +324,61 @@ export function legacyListenToEvent(
 export function legacyListenToTopLevelEvent(
   topLevelType: DOMTopLevelEventType,
   mountAt: Document | Element,
-  listenerMap: Map<DOMTopLevelEventType | string, null | (any => void)>,
+  listenerMap: ElementListenerMap,
 ): void {
   if (!listenerMap.has(topLevelType)) {
     switch (topLevelType) {
-      case TOP_SCROLL:
-        legacyTrapCapturedEvent(TOP_SCROLL, mountAt);
+      case TOP_SCROLL: {
+        legacyTrapCapturedEvent(TOP_SCROLL, mountAt, listenerMap);
         break;
+      }
       case TOP_FOCUS:
       case TOP_BLUR:
-        legacyTrapCapturedEvent(TOP_FOCUS, mountAt);
-        legacyTrapCapturedEvent(TOP_BLUR, mountAt);
-        // We set the flag for a single dependency later in this function,
-        // but this ensures we mark both as attached rather than just one.
-        listenerMap.set(TOP_BLUR, null);
-        listenerMap.set(TOP_FOCUS, null);
+        legacyTrapCapturedEvent(TOP_FOCUS, mountAt, listenerMap);
+        legacyTrapCapturedEvent(TOP_BLUR, mountAt, listenerMap);
         break;
       case TOP_CANCEL:
-      case TOP_CLOSE:
+      case TOP_CLOSE: {
         if (isEventSupported(getRawEventName(topLevelType))) {
-          legacyTrapCapturedEvent(topLevelType, mountAt);
+          legacyTrapCapturedEvent(topLevelType, mountAt, listenerMap);
         }
         break;
+      }
       case TOP_INVALID:
       case TOP_SUBMIT:
       case TOP_RESET:
         // We listen to them on the target DOM elements.
         // Some of them bubble so we don't want them to fire twice.
         break;
-      default:
+      default: {
         // By default, listen on the top level to all non-media events.
         // Media events don't bubble so adding the listener wouldn't do anything.
         const isMediaEvent = mediaEventTypes.indexOf(topLevelType) !== -1;
         if (!isMediaEvent) {
-          legacyTrapBubbledEvent(topLevelType, mountAt);
+          legacyTrapBubbledEvent(topLevelType, mountAt, listenerMap);
         }
         break;
+      }
     }
-    listenerMap.set(topLevelType, null);
   }
 }
 
 export function legacyTrapBubbledEvent(
   topLevelType: DOMTopLevelEventType,
   element: Document | Element,
+  listenerMap?: ElementListenerMap,
 ): void {
-  addTrappedEventListener(element, topLevelType, false);
+  const listener = addTrappedEventListener(element, topLevelType, false);
+  if (listenerMap) {
+    listenerMap.set(topLevelType, {passive: undefined, listener});
+  }
 }
 
 export function legacyTrapCapturedEvent(
   topLevelType: DOMTopLevelEventType,
   element: Document | Element,
+  listenerMap: ElementListenerMap,
 ): void {
-  addTrappedEventListener(element, topLevelType, true);
+  const listener = addTrappedEventListener(element, topLevelType, true);
+  listenerMap.set(topLevelType, {passive: undefined, listener});
 }

packages/react-dom/src/events/DOMModernPluginEventSystem.js

@@ -9,7 +9,10 @@
 
 import type {AnyNativeEvent} from 'legacy-events/PluginModuleType';
 import type {DOMTopLevelEventType} from 'legacy-events/TopLevelEventTypes';
-import type {ElementListenerMap} from '../events/DOMEventListenerMap';
+import type {
+  ElementListenerMap,
+  ElementListenerMapEntry,
+} from '../events/DOMEventListenerMap';
 import type {EventSystemFlags} from 'legacy-events/EventSystemFlags';
 import type {EventPriority} from 'shared/ReactTypes';
 import type {Fiber} from 'react-reconciler/src/ReactFiber';
@@ -24,7 +27,10 @@ import {plugins} from 'legacy-events/EventPluginRegistry';
 
 import {HostRoot, HostPortal} from 'shared/ReactWorkTags';
 
-import {addTrappedEventListener} from './ReactDOMEventListener';
+import {
+  addTrappedEventListener,
+  removeTrappedEventListener,
+} from './ReactDOMEventListener';
 import getEventTarget from './getEventTarget';
 import {getListenerMapForElement} from './DOMEventListenerMap';
 import {
@@ -153,18 +159,47 @@ function dispatchEventsForPlugins(
   }
 }
 
+function shouldUpgradeListener(
+  listenerEntry: void | ElementListenerMapEntry,
+  passive: void | boolean,
+): boolean {
+  if (listenerEntry === undefined) {
+    return false;
+  }
+  // Upgrade from passive to active.
+  if (passive !== true && listenerEntry.passive) {
+    return true;
+  }
+  // Upgrade from default-active (browser default) to active.
+  if (passive === false && listenerEntry.passive === undefined) {
+    return true;
+  }
+  // Otherwise, do not upgrade
+  return false;
+}
+
 export function listenToTopLevelEvent(
   topLevelType: DOMTopLevelEventType,
   targetContainer: EventTarget,
   listenerMap: ElementListenerMap,
   passive?: boolean,
   priority?: EventPriority,
 ): void {
-  // TODO: we need to know if the listenerMap previously was passive
-  // and to check if we need to upgrade to active. This will come in
-  // a useEvent follow up PR.
-  if (!listenerMap.has(topLevelType)) {
+  const listenerEntry = listenerMap.get(topLevelType);
+  const shouldUpgrade = shouldUpgradeListener(listenerEntry, passive);
+  if (listenerEntry === undefined || shouldUpgrade) {
     const isCapturePhase = capturePhaseEvents.has(topLevelType);
+    // If we should upgrade, then we need to remove the existing trapped
+    // event listener for the target container.
+    if (shouldUpgrade) {
+      removeTrappedEventListener(
+        targetContainer,
+        topLevelType,
+        isCapturePhase,
+        ((listenerEntry: any): ElementListenerMapEntry).listener,
+        ((listenerEntry: any): ElementListenerMapEntry).passive,
+      );
+    }
     const listener = addTrappedEventListener(
       targetContainer,
       topLevelType,
@@ -173,7 +208,7 @@ export function listenToTopLevelEvent(
       passive,
       priority,
     );
-    listenerMap.set(topLevelType, listener);
+    listenerMap.set(topLevelType, {passive, listener});
   }
 }
 

packages/react-dom/src/events/ReactDOMEventListener.js

@@ -255,20 +255,22 @@ export function addTrappedEventListener(
 
 export function removeTrappedEventListener(
   targetContainer: EventTarget,
-  topLevelType: string,
+  topLevelType: DOMTopLevelEventType,
+  capture: boolean,
   listener: any => void,
-  passive: boolean,
+  passive: void | boolean,
 ) {
   if (listener.remove != null) {
     listener.remove();
   } else {
+    const rawEventName = getRawEventName(topLevelType);
     if (passiveBrowserEventsSupported) {
-      targetContainer.removeEventListener(topLevelType, listener, {
-        capture: true,
+      targetContainer.removeEventListener(rawEventName, listener, {
+        capture,
         passive,
       });
     } else {
-      targetContainer.removeEventListener(topLevelType, listener, true);
+      targetContainer.removeEventListener(rawEventName, listener, capture);
     }
   }
 }

packages/react-dom/src/events/ReactDOMEventReplaying.js

@@ -244,7 +244,7 @@ function trapReplayableEventForDocument(
         topLevelTypeString,
         false,
       );
-      listenerMap.set(activeEventKey, listener);
+      listenerMap.set(activeEventKey, {passive: false, listener});
     }
   }
 }

packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js

@@ -1648,5 +1648,107 @@ describe('DOMModernPluginEventSystem', () => {
       dispatchClickEvent(ref.current);
       expect(log).toEqual([{counter: 1}]);
     });
+
+    it('should correctly work for a basic "click" listener that upgrades', () => {
+      const clickEvent = jest.fn();
+      const buttonRef = React.createRef();
+      const button2Ref = React.createRef();
+
+      function Test2() {
+        const click = ReactDOM.unstable_useEvent('click', {
+          passive: false,
+        });
+
+        React.useEffect(() => {
+          click.setListener(button2Ref.current, clickEvent);
+        });
+
+        return <button ref={button2Ref}>Click me!</button>;
+      }
+
+      function Test({extra}) {
+        const click = ReactDOM.unstable_useEvent('click', {
+          passive: true,
+        });
+
+        React.useEffect(() => {
+          click.setListener(buttonRef.current, clickEvent);
+        });
+
+        return (
+          <>
+            <button ref={buttonRef}>Click me!</button>
+            {extra && <Test2 />}
+          </>
+        );
+      }
+
+      ReactDOM.render(<Test />, container);
+      Scheduler.unstable_flushAll();
+
+      let button = buttonRef.current;
+      dispatchClickEvent(button);
+      expect(clickEvent).toHaveBeenCalledTimes(1);
+
+      ReactDOM.render(<Test extra={true} />, container);
+      Scheduler.unstable_flushAll();
+
+      clickEvent.mockClear();
+
+      button = button2Ref.current;
+      dispatchClickEvent(button);
+      expect(clickEvent).toHaveBeenCalledTimes(1);
+    });
+
+    it('should correctly work for a basic "click" listener that upgrades #2', () => {
+      const clickEvent = jest.fn();
+      const buttonRef = React.createRef();
+      const button2Ref = React.createRef();
+
+      function Test2() {
+        const click = ReactDOM.unstable_useEvent('click', {
+          passive: false,
+        });
+
+        React.useEffect(() => {
+          click.setListener(button2Ref.current, clickEvent);
+        });
+
+        return <button ref={button2Ref}>Click me!</button>;
+      }
+
+      function Test({extra}) {
+        const click = ReactDOM.unstable_useEvent('click', {
+          passive: undefined,
+        });
+
+        React.useEffect(() => {
+          click.setListener(buttonRef.current, clickEvent);
+        });
+
+        return (
+          <>
+            <button ref={buttonRef}>Click me!</button>
+            {extra && <Test2 />}
+          </>
+        );
+      }
+
+      ReactDOM.render(<Test />, container);
+      Scheduler.unstable_flushAll();
+
+      let button = buttonRef.current;
+      dispatchClickEvent(button);
+      expect(clickEvent).toHaveBeenCalledTimes(1);
+
+      ReactDOM.render(<Test extra={true} />, container);
+      Scheduler.unstable_flushAll();
+
+      clickEvent.mockClear();
+
+      button = button2Ref.current;
+      dispatchClickEvent(button);
+      expect(clickEvent).toHaveBeenCalledTimes(1);
+    });
   });
 });