diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index c356ed4f38e59..be043650bc4cb 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -3211,12 +3211,20 @@ function recursivelyTraverseAndDoubleInvokeEffectsInDEV( } let child = parentFiber.child; while (child !== null) { - doubleInvokeEffectsInDEV(root, child, isInStrictMode); + doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode); child = child.sibling; } } -function doubleInvokeEffectsInDEV( +// Unconditionally disconnects and connects passive and layout effects. +function doubleInvokeEffectsOnFiber(root: FiberRoot, fiber: Fiber) { + disappearLayoutEffects(fiber); + disconnectPassiveEffect(fiber); + reappearLayoutEffects(root, fiber.alternate, fiber, false); + reconnectPassiveEffects(root, fiber, NoLanes, null, false); +} + +function doubleInvokeEffectsInDEVIfNecessary( root: FiberRoot, fiber: Fiber, parentIsInStrictMode: boolean, @@ -3224,22 +3232,43 @@ function doubleInvokeEffectsInDEV( const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE; const isInStrictMode = parentIsInStrictMode || isStrictModeFiber; - if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) { + // First case: the fiber **is not** of type OffscreenComponent. No + // special rules apply to double invoking effects. + if (fiber.tag !== OffscreenComponent) { + if (fiber.flags & PlacementDEV) { + setCurrentDebugFiberInDEV(fiber); + if (isInStrictMode) { + doubleInvokeEffectsOnFiber(root, fiber); + } + resetCurrentDebugFiberInDEV(); + } else { + recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root, + fiber, + isInStrictMode, + ); + } + return; + } + + // Second case: the fiber **is** of type OffscreenComponent. + // This branch contains cases specific to Offscreen. + if (fiber.memoizedState === null) { setCurrentDebugFiberInDEV(fiber); - const isNotOffscreen = fiber.tag !== OffscreenComponent; - // Checks if Offscreen is being revealed. For all other components, evaluates to true. - const hasOffscreenBecomeVisible = - isNotOffscreen || - (fiber.flags & Visibility && fiber.memoizedState === null); - if (isInStrictMode && hasOffscreenBecomeVisible) { - disappearLayoutEffects(fiber); - disconnectPassiveEffect(fiber); - reappearLayoutEffects(root, fiber.alternate, fiber, false); - reconnectPassiveEffects(root, fiber, NoLanes, null, false); + if (isInStrictMode && fiber.flags & Visibility) { + // Double invoke effects on Offscreen's subtree only + // if it is visible and its visibility has changed. + doubleInvokeEffectsOnFiber(root, fiber); + } else if (fiber.subtreeFlags & PlacementDEV) { + // Something in the subtree could have been suspended. + // We need to continue traversal and find newly inserted fibers. + recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root, + fiber, + isInStrictMode, + ); } resetCurrentDebugFiberInDEV(); - } else { - recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode); } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 6250330b6e957..28bed56257c0a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -3211,12 +3211,20 @@ function recursivelyTraverseAndDoubleInvokeEffectsInDEV( } let child = parentFiber.child; while (child !== null) { - doubleInvokeEffectsInDEV(root, child, isInStrictMode); + doubleInvokeEffectsInDEVIfNecessary(root, child, isInStrictMode); child = child.sibling; } } -function doubleInvokeEffectsInDEV( +// Unconditionally disconnects and connects passive and layout effects. +function doubleInvokeEffectsOnFiber(root: FiberRoot, fiber: Fiber) { + disappearLayoutEffects(fiber); + disconnectPassiveEffect(fiber); + reappearLayoutEffects(root, fiber.alternate, fiber, false); + reconnectPassiveEffects(root, fiber, NoLanes, null, false); +} + +function doubleInvokeEffectsInDEVIfNecessary( root: FiberRoot, fiber: Fiber, parentIsInStrictMode: boolean, @@ -3224,22 +3232,43 @@ function doubleInvokeEffectsInDEV( const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE; const isInStrictMode = parentIsInStrictMode || isStrictModeFiber; - if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) { + // First case: the fiber **is not** of type OffscreenComponent. No + // special rules apply to double invoking effects. + if (fiber.tag !== OffscreenComponent) { + if (fiber.flags & PlacementDEV) { + setCurrentDebugFiberInDEV(fiber); + if (isInStrictMode) { + doubleInvokeEffectsOnFiber(root, fiber); + } + resetCurrentDebugFiberInDEV(); + } else { + recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root, + fiber, + isInStrictMode, + ); + } + return; + } + + // Second case: the fiber **is** of type OffscreenComponent. + // This branch contains cases specific to Offscreen. + if (fiber.memoizedState === null) { setCurrentDebugFiberInDEV(fiber); - const isNotOffscreen = fiber.tag !== OffscreenComponent; - // Checks if Offscreen is being revealed. For all other components, evaluates to true. - const hasOffscreenBecomeVisible = - isNotOffscreen || - (fiber.flags & Visibility && fiber.memoizedState === null); - if (isInStrictMode && hasOffscreenBecomeVisible) { - disappearLayoutEffects(fiber); - disconnectPassiveEffect(fiber); - reappearLayoutEffects(root, fiber.alternate, fiber, false); - reconnectPassiveEffects(root, fiber, NoLanes, null, false); + if (isInStrictMode && fiber.flags & Visibility) { + // Double invoke effects on Offscreen's subtree only + // if it is visible and its visibility has changed. + doubleInvokeEffectsOnFiber(root, fiber); + } else if (fiber.subtreeFlags & PlacementDEV) { + // Something in the subtree could have been suspended. + // We need to continue traversal and find newly inserted fibers. + recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root, + fiber, + isInStrictMode, + ); } resetCurrentDebugFiberInDEV(); - } else { - recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode); } } diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js index e6ab08cef5471..32c4bb4cf3955 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js @@ -153,4 +153,77 @@ describe('ReactOffscreenStrictMode', () => { ); }); }); + + // @gate __DEV__ && enableStrictEffects && enableOffscreen + it('child suspense double invoke test', async () => { + let shouldSuspend = true; + let resolve; + const suspensePromise = new Promise(_resolve => { + resolve = _resolve; + }); + + function Parent() { + log.push('Parent rendered'); + React.useEffect(() => { + log.push('Parent mount'); + return () => { + log.push('Parent unmount'); + }; + }); + + return ( + + + + ); + } + + function Child() { + log.push('Child rendered'); + React.useEffect(() => { + log.push('Child mount'); + return () => { + log.push('Child unmount'); + }; + }); + if (shouldSuspend) { + log.push('Child suspended'); + throw suspensePromise; + } + return null; + } + + act(() => { + ReactNoop.render( + + + + + , + ); + }); + + log.push('------------------------------'); + + await act(async () => { + resolve(); + shouldSuspend = false; + }); + + expect(log).toEqual([ + 'Parent rendered', + 'Parent rendered', + 'Child rendered', + 'Child suspended', + 'Parent mount', + 'Parent unmount', + 'Parent mount', + '------------------------------', + 'Child rendered', + 'Child rendered', + 'Child mount', + 'Child unmount', + 'Child mount', + ]); + }); });