Remove Set bookkeeping for root events

packages/react-dom/src/__tests__/ReactDOMEventListener-test.js

@@ -820,19 +820,21 @@ describe('ReactDOMEventListener', () => {
         </div>,
         container,
       );
+
+      // Update to attach.
       ReactDOM.render(
         <div
           className="grand"
-          onScroll={onScroll}
-          onScrollCapture={onScrollCapture}>
+          onScroll={e => onScroll(e)}
+          onScrollCapture={e => onScrollCapture(e)}>
           <div
             className="parent"
-            onScroll={onScroll}
-            onScrollCapture={onScrollCapture}>
+            onScroll={e => onScroll(e)}
+            onScrollCapture={e => onScrollCapture(e)}>
             <div
               className="child"
-              onScroll={onScroll}
-              onScrollCapture={onScrollCapture}
+              onScroll={e => onScroll(e)}
+              onScrollCapture={e => onScrollCapture(e)}
               ref={ref}
             />
           </div>
@@ -850,6 +852,58 @@ describe('ReactDOMEventListener', () => {
         ['capture', 'child'],
         ['bubble', 'child'],
       ]);
+
+      // Update to verify deduplication.
+      log.length = 0;
+      ReactDOM.render(
+        <div
+          className="grand"
+          // Note: these are intentionally inline functions so that
+          // we hit the reattachment codepath instead of bailing out.
+          onScroll={e => onScroll(e)}
+          onScrollCapture={e => onScrollCapture(e)}>
+          <div
+            className="parent"
+            onScroll={e => onScroll(e)}
+            onScrollCapture={e => onScrollCapture(e)}>
+            <div
+              className="child"
+              onScroll={e => onScroll(e)}
+              onScrollCapture={e => onScrollCapture(e)}
+              ref={ref}
+            />
+          </div>
+        </div>,
+        container,
+      );
+      ref.current.dispatchEvent(
+        new Event('scroll', {
+          bubbles: false,
+        }),
+      );
+      expect(log).toEqual([
+        ['capture', 'grand'],
+        ['capture', 'parent'],
+        ['capture', 'child'],
+        ['bubble', 'child'],
+      ]);
+
+      // Update to detach.
+      log.length = 0;
+      ReactDOM.render(
+        <div>
+          <div>
+            <div ref={ref} />
+          </div>
+        </div>,
+        container,
+      );
+      ref.current.dispatchEvent(
+        new Event('scroll', {
+          bubbles: false,
+        }),
+      );
+      expect(log).toEqual([]);
     } finally {
       document.body.removeChild(container);
     }
@@ -899,8 +953,49 @@ describe('ReactDOMEventListener', () => {
         ['capture', 'child'],
         ['bubble', 'child'],
       ]);
+
+      log.length = 0;
+      ReactDOM.render(
+        <div>
+          <div>
+            <div ref={ref} />
+          </div>
+        </div>,
+        container,
+      );
+      ref.current.dispatchEvent(
+        new Event('scroll', {
+          bubbles: false,
+        }),
+      );
+      expect(log).toEqual([]);
     } finally {
       document.body.removeChild(container);
     }
   });
+
+  it('should not subscribe to selectionchange twice', () => {
+    const log = [];
+
+    const originalDocAddEventListener = document.addEventListener;
+    document.addEventListener = function(type, fn, options) {
+      switch (type) {
+        case 'selectionchange':
+          log.push(options);
+          break;
+        default:
+          throw new Error(
+            `Did not expect to add a document-level listener for the "${type}" event.`,
+          );
+      }
+    };
+    try {
+      ReactDOM.render(<input />, document.createElement('div'));
+      ReactDOM.render(<input />, document.createElement('div'));
+    } finally {
+      document.addEventListener = originalDocAddEventListener;
+    }
+
+    expect(log).toEqual([false]);
+  });
 });

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

@@ -22,9 +22,7 @@ import {
   addEventHandleToTarget,
 } from './ReactDOMComponentTree';
 import {ELEMENT_NODE} from '../shared/HTMLNodeType';
-import {listenToNativeEvent} from '../events/DOMPluginEventSystem';
-
-import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../events/EventSystemFlags';
+import {listenToNativeEventForNonManagedEventTarget} from '../events/DOMPluginEventSystem';
 
 import {
   enableScopeAPI,
@@ -69,12 +67,10 @@ function registerReactDOMEvent(
     const eventTarget = ((target: any): EventTarget);
     // These are valid event targets, but they are also
     // non-managed React nodes.
-    listenToNativeEvent(
+    listenToNativeEventForNonManagedEventTarget(
       domEventName,
       isCapturePhaseListener,
       eventTarget,
-      null,
-      IS_EVENT_HANDLE_NON_MANAGED_NODE,
     );
   } else {
     invariant(

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

@@ -295,6 +295,15 @@ export function listenToNonDelegatedEvent(
   domEventName: DOMEventName,
   targetElement: Element,
 ): void {
+  if (__DEV__) {
+    if (!nonDelegatedEvents.has(domEventName)) {
+      console.error(
+        'Did not expect a listenToNonDelegatedEvent() call for "%s". ' +
+          'This is a bug in React. Please file an issue.',
+        domEventName,
+      );
+    }
+  }
   const isCapturePhaseListener = false;
   const listenerSet = getEventListenerSet(targetElement);
   const listenerSetKey = getListenerSetKey(
@@ -312,88 +321,46 @@ export function listenToNonDelegatedEvent(
   }
 }
 
-const listeningMarker =
-  '_reactListening' +
-  Math.random()
-    .toString(36)
-    .slice(2);
-
-export function listenToAllSupportedEvents(rootContainerElement: EventTarget) {
-  if ((rootContainerElement: any)[listeningMarker]) {
-    // Performance optimization: don't iterate through events
-    // for the same portal container or root node more than once.
-    // TODO: once we remove the flag, we may be able to also
-    // remove some of the bookkeeping maps used for laziness.
-    return;
-  }
-  (rootContainerElement: any)[listeningMarker] = true;
-  allNativeEvents.forEach(domEventName => {
-    if (!nonDelegatedEvents.has(domEventName)) {
-      listenToNativeEvent(
+export function listenToNativeEvent(
+  domEventName: DOMEventName,
+  isCapturePhaseListener: boolean,
+  target: EventTarget,
+): void {
+  if (__DEV__) {
+    if (nonDelegatedEvents.has(domEventName) && !isCapturePhaseListener) {
+      console.error(
+        'Did not expect a listenToNativeEvent() call for "%s" in the bubble phase. ' +
+          'This is a bug in React. Please file an issue.',
         domEventName,
-        false,
-        ((rootContainerElement: any): Element),
-        null,
       );
     }
-    listenToNativeEvent(
-      domEventName,
-      true,
-      ((rootContainerElement: any): Element),
-      null,
-    );
-  });
+  }
+
+  let eventSystemFlags = 0;
+  if (isCapturePhaseListener) {
+    eventSystemFlags |= IS_CAPTURE_PHASE;
+  }
+  addTrappedEventListener(
+    target,
+    domEventName,
+    eventSystemFlags,
+    isCapturePhaseListener,
+  );
 }
 
-export function listenToNativeEvent(
+// This is only used by createEventHandle when the
+// target is not a DOM element. E.g. window.
+export function listenToNativeEventForNonManagedEventTarget(
   domEventName: DOMEventName,
   isCapturePhaseListener: boolean,
-  rootContainerElement: EventTarget,
-  targetElement: Element | null,
-  eventSystemFlags?: EventSystemFlags = 0,
+  target: EventTarget,
 ): void {
-  let target = rootContainerElement;
-
-  // selectionchange needs to be attached to the document
-  // otherwise it won't capture incoming events that are only
-  // triggered on the document directly.
-  if (
-    domEventName === 'selectionchange' &&
-    (rootContainerElement: any).nodeType !== DOCUMENT_NODE
-  ) {
-    target = (rootContainerElement: any).ownerDocument;
-  }
-  // If the event can be delegated (or is capture phase), we can
-  // register it to the root container. Otherwise, we should
-  // register the event to the target element and mark it as
-  // a non-delegated event.
-  if (
-    targetElement !== null &&
-    !isCapturePhaseListener &&
-    nonDelegatedEvents.has(domEventName)
-  ) {
-    // For all non-delegated events, apart from scroll, we attach
-    // their event listeners to the respective elements that their
-    // events fire on. That means we can skip this step, as event
-    // listener has already been added previously. However, we
-    // special case the scroll event because the reality is that any
-    // element can scroll.
-    // TODO: ideally, we'd eventually apply the same logic to all
-    // events from the nonDelegatedEvents list. Then we can remove
-    // this special case and use the same logic for all events.
-    if (domEventName !== 'scroll') {
-      return;
-    }
-    eventSystemFlags |= IS_NON_DELEGATED;
-    target = targetElement;
-  }
+  let eventSystemFlags = IS_EVENT_HANDLE_NON_MANAGED_NODE;
   const listenerSet = getEventListenerSet(target);
   const listenerSetKey = getListenerSetKey(
     domEventName,
     isCapturePhaseListener,
   );
-  // If the listener entry is empty or we should upgrade, then
-  // we need to trap an event listener onto the target.
   if (!listenerSet.has(listenerSetKey)) {
     if (isCapturePhaseListener) {
       eventSystemFlags |= IS_CAPTURE_PHASE;
@@ -408,6 +375,40 @@ export function listenToNativeEvent(
   }
 }
 
+const listeningMarker =
+  '_reactListening' +
+  Math.random()
+    .toString(36)
+    .slice(2);
+
+export function listenToAllSupportedEvents(rootContainerElement: EventTarget) {
+  if (!(rootContainerElement: any)[listeningMarker]) {
+    (rootContainerElement: any)[listeningMarker] = true;
+    allNativeEvents.forEach(domEventName => {
+      // We handle selectionchange separately because it
+      // doesn't bubble and needs to be on the document.
+      if (domEventName !== 'selectionchange') {
+        if (!nonDelegatedEvents.has(domEventName)) {
+          listenToNativeEvent(domEventName, false, rootContainerElement);
+        }
+        listenToNativeEvent(domEventName, true, rootContainerElement);
+      }
+    });
+    const ownerDocument =
+      (rootContainerElement: any).nodeType === DOCUMENT_NODE
+        ? rootContainerElement
+        : (rootContainerElement: any).ownerDocument;
+    if (ownerDocument !== null) {
+      // The selectionchange event also needs deduplication
+      // but it is attached to the document.
+      if (!(ownerDocument: any)[listeningMarker]) {
+        (ownerDocument: any)[listeningMarker] = true;
+        listenToNativeEvent('selectionchange', false, ownerDocument);
+      }
+    }
+  }
+}
+
 function addTrappedEventListener(
   targetContainer: EventTarget,
   domEventName: DOMEventName,