Move beforeblur phase to prepareForCommit (#18609)

Author: trueadm

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

@@ -73,7 +73,6 @@ import {
   enableUseEventAPI,
   enableScopeAPI,
 } from 'shared/ReactFeatureFlags';
-import {HostComponent} from 'react-reconciler/src/ReactWorkTags';
 import {
   RESPONDER_EVENT_SYSTEM,
   IS_PASSIVE,
@@ -95,6 +94,10 @@ import {
 import {getListenerMapForElement} from '../events/DOMEventListenerMap';
 import {TOP_BEFORE_BLUR, TOP_AFTER_BLUR} from '../events/DOMTopLevelEventTypes';
 
+// TODO: This is an exposed internal, we should move this around
+// so this isn't the case.
+import {isFiberInsideHiddenOrRemovedTree} from 'react-reconciler/src/ReactFiberTreeReflection';
+
 export type ReactListenerEvent = ReactDOMListenerEvent;
 export type ReactListenerMap = ReactDOMListenerMap;
 export type ReactListener = ReactDOMListener;
@@ -250,6 +253,15 @@ export function getPublicInstance(instance: Instance): * {
 export function prepareForCommit(containerInfo: Container): void {
   eventsEnabled = ReactBrowserEventEmitterIsEnabled();
   selectionInformation = getSelectionInformation();
+  if (enableDeprecatedFlareAPI || enableUseEventAPI) {
+    const focusedElem = selectionInformation.focusedElem;
+    if (focusedElem !== null) {
+      const instance = getClosestInstanceFromNode(focusedElem);
+      if (instance !== null && isFiberInsideHiddenOrRemovedTree(instance)) {
+        dispatchBeforeDetachedBlur(focusedElem);
+      }
+    }
+  }
   ReactBrowserEventEmitterSetEnabled(false);
 }
 
@@ -532,18 +544,11 @@ function dispatchBeforeDetachedBlur(target: HTMLElement): void {
     );
   }
   if (enableUseEventAPI) {
-    try {
-      // We need to temporarily enable the event system
-      // to dispatch the "beforeblur" event.
-      ReactBrowserEventEmitterSetEnabled(true);
-      const event = createEvent(TOP_BEFORE_BLUR);
-      // Dispatch "beforeblur" directly on the target,
-      // so it gets picked up by the event system and
-      // can propagate through the React internal tree.
-      target.dispatchEvent(event);
-    } finally {
-      ReactBrowserEventEmitterSetEnabled(false);
-    }
+    const event = createEvent(TOP_BEFORE_BLUR);
+    // Dispatch "beforeblur" directly on the target,
+    // so it gets picked up by the event system and
+    // can propagate through the React internal tree.
+    target.dispatchEvent(event);
   }
 }
 
@@ -571,20 +576,9 @@ function dispatchAfterDetachedBlur(target: HTMLElement): void {
   }
 }
 
-// This is a specific event for the React Flare
-// event system, so event responders can act
-// accordingly to a DOM node being unmounted that
-// previously had active document focus.
 export function beforeRemoveInstance(
   instance: Instance | TextInstance | SuspenseInstance,
 ): void {
-  if (
-    (enableDeprecatedFlareAPI || enableUseEventAPI) &&
-    selectionInformation &&
-    instance === selectionInformation.focusedElem
-  ) {
-    dispatchBeforeDetachedBlur(((instance: any): HTMLElement));
-  }
   if (enableUseEventAPI) {
     // It's unfortunate that we have to do this cleanup, but
     // it's necessary otherwise we will leak the host instances
@@ -674,28 +668,7 @@ export function clearSuspenseBoundaryFromContainer(
   retryIfBlockedOn(container);
 }
 
-function instanceContainsElem(instance: Instance, element: HTMLElement) {
-  let fiber = getClosestInstanceFromNode(element);
-  while (fiber !== null) {
-    if (fiber.tag === HostComponent && fiber.stateNode === instance) {
-      return true;
-    }
-    fiber = fiber.return;
-  }
-  return false;
-}
-
 export function hideInstance(instance: Instance): void {
-  // Ensure we trigger `onBeforeBlur` if the active focused elment
-  // is ether the instance of a child or the instance. We need
-  // to traverse the Fiber tree here rather than use node.contains()
-  // as the child node might be inside a Portal.
-  if ((enableDeprecatedFlareAPI || enableUseEventAPI) && selectionInformation) {
-    const focusedElem = selectionInformation.focusedElem;
-    if (focusedElem !== null && instanceContainsElem(instance, focusedElem)) {
-      dispatchBeforeDetachedBlur(((focusedElem: any): HTMLElement));
-    }
-  }
   // TODO: Does this work for all element types? What about MathML? Should we
   // pass host context to this method?
   instance = ((instance: any): HTMLElement);

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

@@ -23,6 +23,7 @@ const initializeModules = hasPointerEvents => {
   jest.resetModules();
   ReactFeatureFlags = require('shared/ReactFeatureFlags');
   ReactFeatureFlags.enableDeprecatedFlareAPI = true;
+  ReactFeatureFlags.enableScopeAPI = true;
   React = require('react');
   ReactDOM = require('react-dom');
   Scheduler = require('scheduler');
@@ -364,6 +365,40 @@ describe.each(table)('FocusWithin responder', hasPointerEvents => {
       );
     });
 
+    // @gate experimental
+    it('is called after a nested focused element is unmounted (with scope query)', () => {
+      const TestScope = React.unstable_createScope();
+      const testScopeQuery = (type, props) => true;
+      let targetNodes;
+      let targetNode;
+
+      const Component = ({show}) => {
+        const scopeRef = React.useRef(null);
+        const listener = useFocusWithin({
+          onBeforeBlurWithin(event) {
+            const scope = scopeRef.current;
+            targetNode = innerRef.current;
+            targetNodes = scope.DO_NOT_USE_queryAllNodes(testScopeQuery);
+          },
+        });
+
+        return (
+          <TestScope ref={scopeRef} DEPRECATED_flareListeners={[listener]}>
+            {show && <input ref={innerRef} />}
+          </TestScope>
+        );
+      };
+
+      ReactDOM.render(<Component show={true} />, container);
+
+      const inner = innerRef.current;
+      const target = createEventTarget(inner);
+      target.keydown({key: 'Tab'});
+      target.focus();
+      ReactDOM.render(<Component show={false} />, container);
+      expect(targetNodes).toEqual([targetNode]);
+    });
+
     // @gate experimental
     it('is called after a focused suspended element is hidden', () => {
       const Suspense = React.Suspense;

packages/react-reconciler/src/ReactFiberCommitWork.new.js

@@ -1116,17 +1116,21 @@ function detachFiber(fiber: Fiber) {
   // get GC:ed but we don't know which for sure which parent is the current
   // one so we'll settle for GC:ing the subtree of this child. This child
   // itself will be GC:ed when the parent updates the next time.
-  fiber.return = null;
+  fiber.alternate = null;
   fiber.child = null;
-  fiber.memoizedState = null;
-  fiber.updateQueue = null;
   fiber.dependencies = null;
-  fiber.alternate = null;
   fiber.firstEffect = null;
   fiber.lastEffect = null;
-  fiber.pendingProps = null;
   fiber.memoizedProps = null;
+  fiber.memoizedState = null;
+  fiber.pendingProps = null;
+  fiber.return = null;
+  fiber.sibling = null;
   fiber.stateNode = null;
+  fiber.updateQueue = null;
+  if (__DEV__) {
+    fiber._debugOwner = null;
+  }
 }
 
 function emptyPortalContainer(current: Fiber) {

packages/react-reconciler/src/ReactFiberCommitWork.old.js

@@ -1116,17 +1116,21 @@ function detachFiber(fiber: Fiber) {
   // get GC:ed but we don't know which for sure which parent is the current
   // one so we'll settle for GC:ing the subtree of this child. This child
   // itself will be GC:ed when the parent updates the next time.
-  fiber.return = null;
+  fiber.alternate = null;
   fiber.child = null;
-  fiber.memoizedState = null;
-  fiber.updateQueue = null;
   fiber.dependencies = null;
-  fiber.alternate = null;
   fiber.firstEffect = null;
   fiber.lastEffect = null;
-  fiber.pendingProps = null;
   fiber.memoizedProps = null;
+  fiber.memoizedState = null;
+  fiber.pendingProps = null;
+  fiber.return = null;
+  fiber.sibling = null;
   fiber.stateNode = null;
+  fiber.updateQueue = null;
+  if (__DEV__) {
+    fiber._debugOwner = null;
+  }
 }
 
 function emptyPortalContainer(current: Fiber) {

packages/react-reconciler/src/ReactFiberScope.new.js

@@ -17,24 +17,11 @@ import type {
 } from 'shared/ReactTypes';
 
 import {getPublicInstance, getInstanceFromNode} from './ReactFiberHostConfig';
+import {isFiberSuspenseAndTimedOut} from './ReactFiberTreeReflection';
 
-import {
-  HostComponent,
-  SuspenseComponent,
-  ScopeComponent,
-  ContextProvider,
-} from './ReactWorkTags';
+import {HostComponent, ScopeComponent, ContextProvider} from './ReactWorkTags';
 import {enableScopeAPI} from 'shared/ReactFeatureFlags';
 
-function isFiberSuspenseAndTimedOut(fiber: Fiber): boolean {
-  const memoizedState = fiber.memoizedState;
-  return (
-    fiber.tag === SuspenseComponent &&
-    memoizedState !== null &&
-    memoizedState.dehydrated === null
-  );
-}
-
 function getSuspenseFallbackChild(fiber: Fiber): Fiber | null {
   return ((((fiber.child: any): Fiber).sibling: any): Fiber).child;
 }

packages/react-reconciler/src/ReactFiberScope.old.js

@@ -17,24 +17,11 @@ import type {
 } from 'shared/ReactTypes';
 
 import {getPublicInstance, getInstanceFromNode} from './ReactFiberHostConfig';
+import {isFiberSuspenseAndTimedOut} from './ReactFiberTreeReflection';
 
-import {
-  HostComponent,
-  SuspenseComponent,
-  ScopeComponent,
-  ContextProvider,
-} from './ReactWorkTags';
+import {HostComponent, ScopeComponent, ContextProvider} from './ReactWorkTags';
 import {enableScopeAPI} from 'shared/ReactFeatureFlags';
 
-function isFiberSuspenseAndTimedOut(fiber: Fiber): boolean {
-  const memoizedState = fiber.memoizedState;
-  return (
-    fiber.tag === SuspenseComponent &&
-    memoizedState !== null &&
-    memoizedState.dehydrated === null
-  );
-}
-
 function getSuspenseFallbackChild(fiber: Fiber): Fiber | null {
   return ((((fiber.child: any): Fiber).sibling: any): Fiber).child;
 }

packages/react-reconciler/src/ReactFiberTreeReflection.js

@@ -25,7 +25,7 @@ import {
   FundamentalComponent,
   SuspenseComponent,
 } from './ReactWorkTags';
-import {NoEffect, Placement, Hydrating} from './ReactSideEffectTags';
+import {NoEffect, Placement, Hydrating, Deletion} from './ReactSideEffectTags';
 import {enableFundamentalAPI} from 'shared/ReactFeatureFlags';
 
 const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
@@ -332,3 +332,25 @@ export function findCurrentHostFiberWithNoPortals(parent: Fiber): Fiber | null {
   // eslint-disable-next-line no-unreachable
   return null;
 }
+
+export function isFiberSuspenseAndTimedOut(fiber: Fiber): boolean {
+  const memoizedState = fiber.memoizedState;
+  return (
+    fiber.tag === SuspenseComponent &&
+    memoizedState !== null &&
+    memoizedState.dehydrated === null
+  );
+}
+
+// This is only safe to call in the commit phase when the return tree is consistent.
+// It should not be used anywhere else. See PR #18609 for details.
+export function isFiberInsideHiddenOrRemovedTree(fiber: Fiber): boolean {
+  let node = fiber;
+  while (node !== null) {
+    if (node.effectTag & Deletion || isFiberSuspenseAndTimedOut(node)) {
+      return true;
+    }
+    node = node.return;
+  }
+  return false;
+}