diff --git a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js index 773ab97b063a4..454ba65a416d0 100644 --- a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js @@ -2489,6 +2489,68 @@ describe('DOMModernPluginEventSystem', () => { document.body.removeChild(container2); }); + // @gate experimental + it('regression: does not fire beforeblur/afterblur if target is already hidden', () => { + const Suspense = React.Suspense; + let suspend = false; + const promise = Promise.resolve(); + const beforeBlurHandle = ReactDOM.unstable_createEventHandle( + 'beforeblur', + ); + const innerRef = React.createRef(); + + function Child() { + if (suspend) { + throw promise; + } + return ; + } + + const Component = () => { + const ref = React.useRef(null); + const [, setState] = React.useState(0); + + React.useEffect(() => { + beforeBlurHandle.setListener(ref.current, () => { + // In the regression case, this would trigger an update, then + // the resulting render would trigger another blur event, + // which would trigger an update again, and on and on in an + // infinite loop. + setState(n => n + 1); + }); + }, []); + + return ( +
+ + + +
+ ); + }; + + const container2 = document.createElement('div'); + document.body.appendChild(container2); + + const root = ReactDOM.createRoot(container2); + ReactTestUtils.act(() => { + root.render(); + }); + + // Focus the input node + const inner = innerRef.current; + const target = createEventTarget(inner); + target.focus(); + + // Suspend. This hides the input node, causing it to lose focus. + suspend = true; + ReactTestUtils.act(() => { + root.render(); + }); + + document.body.removeChild(container2); + }); + describe('Compatibility with Scopes API', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 5e843a70d8cd1..e7bc770127c9d 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1752,6 +1752,23 @@ function attachSuspenseRetryListeners(finishedWork: Fiber) { } } +// This function detects when a Suspense boundary goes from visible to hidden. +// It returns false if the boundary is already hidden. +// TODO: Use an effect tag. +export function isSuspenseBoundaryBeingHidden( + current: Fiber | null, + finishedWork: Fiber, +): boolean { + if (current !== null) { + const oldState: SuspenseState | null = current.memoizedState; + if (oldState === null || oldState.dehydrated !== null) { + const newState: SuspenseState | null = finishedWork.memoizedState; + return newState !== null && newState.dehydrated === null; + } + } + return false; +} + function commitResetTextContent(current: Fiber) { if (!supportsMutation) { return; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 229eab40ee825..7c5e355091385 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1735,6 +1735,23 @@ function attachSuspenseRetryListeners(finishedWork: Fiber) { } } +// This function detects when a Suspense boundary goes from visible to hidden. +// It returns false if the boundary is already hidden. +// TODO: Use an effect tag. +export function isSuspenseBoundaryBeingHidden( + current: Fiber | null, + finishedWork: Fiber, +): boolean { + if (current !== null) { + const oldState: SuspenseState | null = current.memoizedState; + if (oldState === null || oldState.dehydrated !== null) { + const newState: SuspenseState | null = finishedWork.memoizedState; + return newState !== null && newState.dehydrated === null; + } + } + return false; +} + function commitResetTextContent(current: Fiber) { if (!supportsMutation) { return; diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index 1a8be6e497144..ed63a099f4279 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -25,7 +25,7 @@ import { FundamentalComponent, SuspenseComponent, } from './ReactWorkTags'; -import {NoEffect, Placement, Hydrating, Deletion} from './ReactSideEffectTags'; +import {NoEffect, Placement, Hydrating} from './ReactSideEffectTags'; import {enableFundamentalAPI} from 'shared/ReactFeatureFlags'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -342,7 +342,10 @@ export function isFiberSuspenseAndTimedOut(fiber: Fiber): boolean { ); } -function doesFiberContain(parentFiber: Fiber, childFiber: Fiber): boolean { +export function doesFiberContain( + parentFiber: Fiber, + childFiber: Fiber, +): boolean { let node = childFiber; const parentFiberAlternate = parentFiber.alternate; while (node !== null) { @@ -353,34 +356,3 @@ function doesFiberContain(parentFiber: Fiber, childFiber: Fiber): boolean { } return false; } - -function isFiberTimedOutSuspenseThatContainsTargetFiber( - fiber: Fiber, - targetFiber: Fiber, -): boolean { - const child = fiber.child; - return ( - isFiberSuspenseAndTimedOut(fiber) && - child !== null && - doesFiberContain(child, targetFiber) - ); -} - -function isFiberDeletedAndContainsTargetFiber( - fiber: Fiber, - targetFiber: Fiber, -): boolean { - return ( - (fiber.effectTag & Deletion) !== 0 && doesFiberContain(fiber, targetFiber) - ); -} - -export function isFiberHiddenOrDeletedAndContains( - parentFiber: Fiber, - childFiber: Fiber, -): boolean { - return ( - isFiberDeletedAndContainsTargetFiber(parentFiber, childFiber) || - isFiberTimedOutSuspenseThatContainsTargetFiber(parentFiber, childFiber) - ); -} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index c8bacdb12cd65..1958a450a3b77 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -159,6 +159,7 @@ import { commitAttachRef, commitPassiveEffectDurations, commitResetTextContent, + isSuspenseBoundaryBeingHidden, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactUpdateQueue.new'; import {resetContextDependencies} from './ReactFiberNewContext.new'; @@ -201,7 +202,7 @@ import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors'; // Used by `act` import enqueueTask from 'shared/enqueueTask'; -import {isFiberHiddenOrDeletedAndContains} from './ReactFiberTreeReflection'; +import {doesFiberContain} from './ReactFiberTreeReflection'; const ceil = Math.ceil; @@ -2102,19 +2103,31 @@ function commitRootImpl(root, renderPriorityLevel) { function commitBeforeMutationEffects() { while (nextEffect !== null) { - if ( - !shouldFireAfterActiveInstanceBlur && - focusedInstanceHandle !== null && - isFiberHiddenOrDeletedAndContains(nextEffect, focusedInstanceHandle) - ) { - shouldFireAfterActiveInstanceBlur = true; - beforeActiveInstanceBlur(); + const current = nextEffect.alternate; + + if (!shouldFireAfterActiveInstanceBlur && focusedInstanceHandle !== null) { + if ((nextEffect.effectTag & Deletion) !== NoEffect) { + if (doesFiberContain(nextEffect, focusedInstanceHandle)) { + shouldFireAfterActiveInstanceBlur = true; + beforeActiveInstanceBlur(); + } + } else { + // TODO: Move this out of the hot path using a dedicated effect tag. + if ( + nextEffect.tag === SuspenseComponent && + isSuspenseBoundaryBeingHidden(current, nextEffect) && + doesFiberContain(nextEffect, focusedInstanceHandle) + ) { + shouldFireAfterActiveInstanceBlur = true; + beforeActiveInstanceBlur(); + } + } } + const effectTag = nextEffect.effectTag; if ((effectTag & Snapshot) !== NoEffect) { setCurrentDebugFiberInDEV(nextEffect); - const current = nextEffect.alternate; commitBeforeMutationEffectOnFiber(current, nextEffect); resetCurrentDebugFiberInDEV(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 908108f972650..313e4afdde657 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -156,6 +156,7 @@ import { commitAttachRef, commitPassiveEffectDurations, commitResetTextContent, + isSuspenseBoundaryBeingHidden, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactUpdateQueue.old'; import {resetContextDependencies} from './ReactFiberNewContext.old'; @@ -193,7 +194,7 @@ import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors'; // Used by `act` import enqueueTask from 'shared/enqueueTask'; -import {isFiberHiddenOrDeletedAndContains} from './ReactFiberTreeReflection'; +import {doesFiberContain} from './ReactFiberTreeReflection'; const ceil = Math.ceil; @@ -2220,19 +2221,31 @@ function commitRootImpl(root, renderPriorityLevel) { function commitBeforeMutationEffects() { while (nextEffect !== null) { - if ( - !shouldFireAfterActiveInstanceBlur && - focusedInstanceHandle !== null && - isFiberHiddenOrDeletedAndContains(nextEffect, focusedInstanceHandle) - ) { - shouldFireAfterActiveInstanceBlur = true; - beforeActiveInstanceBlur(); + const current = nextEffect.alternate; + + if (!shouldFireAfterActiveInstanceBlur && focusedInstanceHandle !== null) { + if ((nextEffect.effectTag & Deletion) !== NoEffect) { + if (doesFiberContain(nextEffect, focusedInstanceHandle)) { + shouldFireAfterActiveInstanceBlur = true; + beforeActiveInstanceBlur(); + } + } else { + // TODO: Move this out of the hot path using a dedicated effect tag. + if ( + nextEffect.tag === SuspenseComponent && + isSuspenseBoundaryBeingHidden(current, nextEffect) && + doesFiberContain(nextEffect, focusedInstanceHandle) + ) { + shouldFireAfterActiveInstanceBlur = true; + beforeActiveInstanceBlur(); + } + } } + const effectTag = nextEffect.effectTag; if ((effectTag & Snapshot) !== NoEffect) { setCurrentDebugFiberInDEV(nextEffect); - const current = nextEffect.alternate; commitBeforeMutationEffectOnFiber(current, nextEffect); resetCurrentDebugFiberInDEV();