Address feedback

Address feedback

Address feedback

Address feedback #2

Adjust changes

Author: trueadm

packages/legacy-events/PluginModuleType.js

@@ -29,7 +29,7 @@ export type PluginModule<NativeEvent> = {
     nativeTarget: NativeEvent,
     nativeEventTarget: null | EventTarget,
     eventSystemFlags: EventSystemFlags,
-    container?: EventTarget,
+    container?: null | EventTarget,
   ) => ?ReactSyntheticEvent,
   tapMoveThreshold?: number,
 };

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

@@ -69,7 +69,6 @@ import {
   enableUseEventAPI,
 } from 'shared/ReactFeatureFlags';
 import {HostComponent} from 'shared/ReactWorkTags';
-import invariant from 'shared/invariant';
 import {
   RESPONDER_EVENT_SYSTEM,
   IS_PASSIVE,
@@ -1093,35 +1092,19 @@ export function mountEventListener(listener: ReactDOMListener): void {
       // TODO (useEvent)
     } else if (isDOMElement(target)) {
       attachElementListener(listener);
-    } else {
-      // The user should never get to here or we missed something in
-      // validateEventListenerTarget below.
-      invariant(
-        false,
-        'A useEvent listener target instance was not handled. This error is ' +
-          'likely caused by a bug in React. Please file an issue.',
-      );
     }
   }
 }
 
 export function unmountEventListener(listener: ReactDOMListener): void {
   if (enableUseEventAPI) {
     const {target} = listener;
-    if (isDOMElement(target)) {
+    if (target === window) {
       // TODO (useEvent)
     } else if (isDOMDocument(target)) {
       // TODO (useEvent)
     } else if (isDOMElement(target)) {
       detachElementListener(listener);
-    } else {
-      // The user should never get to here or we missed something in
-      // validateEventListenerTarget below.
-      invariant(
-        false,
-        'A useEvent listener target instance was not handled. This error is ' +
-          'likely caused by a bug in React. Please file an issue.',
-      );
     }
   }
 }
@@ -1150,9 +1133,11 @@ export function validateEventListenerTarget(
     }
     if (__DEV__) {
       console.warn(
-        'Event listener method setListener() from useEvent() hook requires the first argument to be either a valid ' +
-          'DOM node that was rendered and managed by React, or either the "window" or "document" objects.' +
-          "If you are setting setListener from ref, ensure the ref's current value has been set and is not null.",
+        'Event listener method setListener() from useEvent() hook requires the first argument to be either:' +
+          '\n\n' +
+          '1. A valid DOM node that was rendered and managed by React' +
+          '2. The "window" object' +
+          '3. The "document" object',
       );
     }
   }

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

@@ -115,7 +115,7 @@ function dispatchEventsForPlugins(
   eventSystemFlags: EventSystemFlags,
   nativeEvent: AnyNativeEvent,
   targetInst: null | Fiber,
-  rootContainer: EventTarget,
+  targetContainer: null | EventTarget,
 ): void {
   const nativeEventTarget = getEventTarget(nativeEvent);
   const syntheticEvents: Array<ReactSyntheticEvent> = [];
@@ -129,7 +129,7 @@ function dispatchEventsForPlugins(
         nativeEvent,
         nativeEventTarget,
         eventSystemFlags,
-        rootContainer,
+        targetContainer,
       );
       if (isArray(extractedEvents)) {
         // Flow complains about @@iterator being missing in ReactSyntheticEvent,
@@ -209,7 +209,7 @@ function willDeferLaterForFBLegacyPrimer(nativeEvent: any): boolean {
       const legacyFBSupport = true;
       const isCapture = nativeEvent.eventPhase === 1;
       addTrappedEventListener(
-        document,
+        null,
         ((type: any): DOMTopLevelEventType),
         isCapture,
         legacyFBSupport,
@@ -223,18 +223,18 @@ function willDeferLaterForFBLegacyPrimer(nativeEvent: any): boolean {
 
 function isMatchingRootContainer(
   grandContainer: Element,
-  rootContainer: EventTarget,
+  targetContainer: EventTarget,
 ): boolean {
   return (
-    grandContainer === rootContainer ||
+    grandContainer === targetContainer ||
     (grandContainer.nodeType === COMMENT_NODE &&
-      grandContainer.parentNode === rootContainer)
+      grandContainer.parentNode === targetContainer)
   );
 }
 
 export function isDOMElement(target: EventTarget): boolean {
   const nodeType = ((target: any): Node).nodeType;
-  return nodeType != null && nodeType === ELEMENT_NODE;
+  return (nodeType: any) && nodeType === ELEMENT_NODE;
 }
 
 export function isDOMDocument(target: EventTarget): boolean {
@@ -247,14 +247,22 @@ export function dispatchEventForPluginEventSystem(
   eventSystemFlags: EventSystemFlags,
   nativeEvent: AnyNativeEvent,
   targetInst: null | Fiber,
-  rootContainer: EventTarget,
+  targetContainer: null | EventTarget,
 ): void {
   let ancestorInst = targetInst;
-  // Given the rootContainer can be any EventTarget, if the
-  // target is not a valid DOM element then we'll skip this part.
-  if (isDOMElement(rootContainer)) {
+  if (targetContainer !== null) {
+    const possibleTargetContainerNode = ((targetContainer: any): Node);
+    // Given the rootContainer can be any EventTarget, if the
+    // target is not a valid DOM element then we'll skip this part.
+    if (
+      possibleTargetContainerNode === window ||
+      !isDOMElement(possibleTargetContainerNode)
+    ) {
+      // TODO: useEvent for document and window
+      return;
+    }
     // If we detect the FB legacy primer system, we
-    // defer the event to the "document" with a one
+    // defer the event to the null with a one
     // time event listener so we can defer the event.
     if (
       enableLegacyFBPrimerSupport &&
@@ -281,7 +289,7 @@ export function dispatchEventForPluginEventSystem(
       }
       if (node.tag === HostRoot || node.tag === HostPortal) {
         const container = node.stateNode.containerInfo;
-        if (isMatchingRootContainer(container, rootContainer)) {
+        if (isMatchingRootContainer(container, possibleTargetContainerNode)) {
           break;
         }
         if (node.tag === HostPortal) {
@@ -293,7 +301,12 @@ export function dispatchEventForPluginEventSystem(
           while (grandNode !== null) {
             if (grandNode.tag === HostRoot || grandNode.tag === HostPortal) {
               const grandContainer = grandNode.stateNode.containerInfo;
-              if (isMatchingRootContainer(grandContainer, rootContainer)) {
+              if (
+                isMatchingRootContainer(
+                  grandContainer,
+                  possibleTargetContainerNode,
+                )
+              ) {
                 // This is the rootContainer we're looking for and we found it as
                 // a parent of the Portal. That means we can ignore it because the
                 // Portal will bubble through to us.
@@ -320,7 +333,7 @@ export function dispatchEventForPluginEventSystem(
       eventSystemFlags,
       nativeEvent,
       ancestorInst,
-      rootContainer,
+      targetContainer,
     ),
   );
 }
@@ -347,11 +360,13 @@ export function attachElementListener(listener: ReactDOMListener): void {
   // find the nearest root/portal contained to attach the event listener
   // to. If it's not managed, i.e. the window, then we just attach
   // the listener to the target.
-  const possibleManagedTarget = ((target: any): Element);
-  if (getClosestInstanceFromNode(possibleManagedTarget)) {
-    containerEventTarget = getNearestRootOrPortalContainer(
-      possibleManagedTarget,
-    );
+  if (isDOMElement(target)) {
+    const possibleManagedTarget = ((target: any): Element);
+    if (getClosestInstanceFromNode(possibleManagedTarget)) {
+      containerEventTarget = getNearestRootOrPortalContainer(
+        possibleManagedTarget,
+      );
+    }
   }
   const listenerMap = getListenerMapForElement(containerEventTarget);
   // Add the event listener to the target container (falling back to

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

@@ -132,7 +132,7 @@ export function addResponderEventSystemEvent(
 }
 
 export function addTrappedEventListener(
-  targetContainer: EventTarget,
+  targetContainer: null | EventTarget,
   topLevelType: DOMTopLevelEventType,
   capture: boolean,
   legacyFBSupport?: boolean,
@@ -165,6 +165,18 @@ export function addTrappedEventListener(
     targetContainer,
   );
 
+  // When the targetContainer is null, it means that the container
+  // target is null, but really we need a real DOM node to attach to.
+  // In this case, we fallback to the "document" node, but leave the
+  // targetContainer (which is bound in the above function) to null.
+  // Really, this only happens for TestUtils.Simulate, so when we
+  // remove that support, we can remove this block of code.
+  if (targetContainer === null) {
+    targetContainer = document;
+  }
+
+  const validTargetContainer = ((targetContainer: any): EventTarget);
+
   const rawEventName = getRawEventName(topLevelType);
   let fbListener;
 
@@ -188,7 +200,7 @@ export function addTrappedEventListener(
         if (fbListener) {
           fbListener.remove();
         } else {
-          targetContainer.removeEventListener(
+          validTargetContainer.removeEventListener(
             ((rawEventName: any): string),
             (listener: any),
           );
@@ -200,14 +212,14 @@ export function addTrappedEventListener(
     if (enableUseEventAPI && passive !== undefined) {
       // This is only used with passive is either true or false.
       fbListener = addEventCaptureListenerWithPassiveFlag(
-        targetContainer,
+        validTargetContainer,
         rawEventName,
         listener,
         passive,
       );
     } else {
       fbListener = addEventCaptureListener(
-        targetContainer,
+        validTargetContainer,
         rawEventName,
         listener,
       );
@@ -216,14 +228,14 @@ export function addTrappedEventListener(
     if (enableUseEventAPI && passive !== undefined) {
       // This is only used with passive is either true or false.
       fbListener = addEventBubbleListenerWithPassiveFlag(
-        targetContainer,
+        validTargetContainer,
         rawEventName,
         listener,
         passive,
       );
     } else {
       fbListener = addEventBubbleListener(
-        targetContainer,
+        validTargetContainer,
         rawEventName,
         listener,
       );
@@ -291,7 +303,7 @@ function dispatchUserBlockingUpdate(
 export function dispatchEvent(
   topLevelType: DOMTopLevelEventType,
   eventSystemFlags: EventSystemFlags,
-  container: EventTarget,
+  targetContainer: null | EventTarget,
   nativeEvent: AnyNativeEvent,
 ): void {
   if (!_enabled) {
@@ -305,7 +317,7 @@ export function dispatchEvent(
       null, // Flags that we're not actually blocked on anything as far as we know.
       topLevelType,
       eventSystemFlags,
-      container,
+      targetContainer,
       nativeEvent,
     );
     return;
@@ -314,7 +326,7 @@ export function dispatchEvent(
   const blockedOn = attemptToDispatchEvent(
     topLevelType,
     eventSystemFlags,
-    container,
+    targetContainer,
     nativeEvent,
   );
 
@@ -330,7 +342,7 @@ export function dispatchEvent(
       blockedOn,
       topLevelType,
       eventSystemFlags,
-      container,
+      targetContainer,
       nativeEvent,
     );
     return;
@@ -341,7 +353,7 @@ export function dispatchEvent(
       blockedOn,
       topLevelType,
       eventSystemFlags,
-      container,
+      targetContainer,
       nativeEvent,
     )
   ) {
@@ -362,7 +374,7 @@ export function dispatchEvent(
           eventSystemFlags,
           nativeEvent,
           null,
-          container,
+          targetContainer,
         );
       } else {
         dispatchEventForLegacyPluginEventSystem(
@@ -390,7 +402,7 @@ export function dispatchEvent(
         eventSystemFlags,
         nativeEvent,
         null,
-        container,
+        targetContainer,
       );
     } else {
       dispatchEventForLegacyPluginEventSystem(
@@ -407,7 +419,7 @@ export function dispatchEvent(
 export function attemptToDispatchEvent(
   topLevelType: DOMTopLevelEventType,
   eventSystemFlags: EventSystemFlags,
-  container: EventTarget,
+  targetContainer: EventTarget | null,
   nativeEvent: AnyNativeEvent,
 ): null | Container | SuspenseInstance {
   // TODO: Warn if _enabled is false.
@@ -461,7 +473,7 @@ export function attemptToDispatchEvent(
           eventSystemFlags,
           nativeEvent,
           targetInst,
-          container,
+          targetContainer,
         );
       } else {
         dispatchEventForLegacyPluginEventSystem(
@@ -489,7 +501,7 @@ export function attemptToDispatchEvent(
         eventSystemFlags,
         nativeEvent,
         targetInst,
-        container,
+        targetContainer,
       );
     } else {
       dispatchEventForLegacyPluginEventSystem(