diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index efb36f2a68147..3f7417fc626d8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -3555,6 +3555,17 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void { function detachAlternateSiblings(parentFiber: Fiber) { if (deletedTreeCleanUpLevel >= 1) { + // A fiber was deleted from this parent fiber, but it's still part of the + // previous (alternate) parent fiber's list of children. Because children + // are a linked list, an earlier sibling that's still alive will be + // connected to the deleted fiber via its `alternate`: + // + // live fiber --alternate--> previous live fiber --sibling--> deleted + // fiber + // + // We can't disconnect `alternate` on nodes that haven't been deleted yet, + // but we can disconnect the `sibling` and `child` pointers. + const previousFiber = parentFiber.alternate; if (previousFiber !== null) { let detachedChild = previousFiber.child; @@ -3613,17 +3624,6 @@ function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void { ); } } - // A fiber was deleted from this parent fiber, but it's still part of - // the previous (alternate) parent fiber's list of children. Because - // children are a linked list, an earlier sibling that's still alive - // will be connected to the deleted fiber via its `alternate`: - // - // live fiber - // --alternate--> previous live fiber - // --sibling--> deleted fiber - // - // We can't disconnect `alternate` on nodes that haven't been deleted - // yet, but we can disconnect the `sibling` and `child` pointers. detachAlternateSiblings(parentFiber); } @@ -3655,11 +3655,32 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { } break; } - // TODO: Disconnect passive effects when a tree is hidden, perhaps after - // a delay. - // case OffscreenComponent: { - // ... - // } + case OffscreenComponent: { + const instance: OffscreenInstance = finishedWork.stateNode; + const nextState: OffscreenState | null = finishedWork.memoizedState; + + const isHidden = nextState !== null; + + if ( + isHidden && + instance.visibility & OffscreenPassiveEffectsConnected && + // For backwards compatibility, don't unmount when a tree suspends. In + // the future we may change this to unmount after a delay. + (finishedWork.return === null || + finishedWork.return.tag !== SuspenseComponent) + ) { + // The effects are currently connected. Disconnect them. + // TODO: Add option or heuristic to delay before disconnecting the + // effects. Then if the tree reappears before the delay has elapsed, we + // can skip toggling the effects entirely. + instance.visibility &= ~OffscreenPassiveEffectsConnected; + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + } else { + recursivelyTraversePassiveUnmountEffects(finishedWork); + } + + break; + } default: { recursivelyTraversePassiveUnmountEffects(finishedWork); break; @@ -3667,6 +3688,70 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { } } +function recursivelyTraverseDisconnectPassiveEffects(parentFiber: Fiber): void { + // Deletions effects can be scheduled on any fiber type. They need to happen + // before the children effects have fired. + const deletions = parentFiber.deletions; + + if ((parentFiber.flags & ChildDeletion) !== NoFlags) { + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const childToDelete = deletions[i]; + // TODO: Convert this to use recursion + nextEffect = childToDelete; + commitPassiveUnmountEffectsInsideOfDeletedTree_begin( + childToDelete, + parentFiber, + ); + } + } + detachAlternateSiblings(parentFiber); + } + + const prevDebugFiber = getCurrentDebugFiberInDEV(); + // TODO: Check PassiveStatic flag + let child = parentFiber.child; + while (child !== null) { + setCurrentDebugFiberInDEV(child); + disconnectPassiveEffect(child); + child = child.sibling; + } + setCurrentDebugFiberInDEV(prevDebugFiber); +} + +function disconnectPassiveEffect(finishedWork: Fiber): void { + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + // TODO: Check PassiveStatic flag + commitHookPassiveUnmountEffects( + finishedWork, + finishedWork.return, + HookPassive, + ); + // When disconnecting passive effects, we fire the effects in the same + // order as during a deletiong: parent before child + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + break; + } + case OffscreenComponent: { + const instance: OffscreenInstance = finishedWork.stateNode; + if (instance.visibility & OffscreenPassiveEffectsConnected) { + instance.visibility &= ~OffscreenPassiveEffectsConnected; + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + } else { + // The effects are already disconnected. + } + break; + } + default: { + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + break; + } + } +} + function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( deletedSubtreeRoot: Fiber, nearestMountedAncestor: Fiber | null, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 553c51f4b0287..b0e16e8ab2aeb 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -3555,6 +3555,17 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void { function detachAlternateSiblings(parentFiber: Fiber) { if (deletedTreeCleanUpLevel >= 1) { + // A fiber was deleted from this parent fiber, but it's still part of the + // previous (alternate) parent fiber's list of children. Because children + // are a linked list, an earlier sibling that's still alive will be + // connected to the deleted fiber via its `alternate`: + // + // live fiber --alternate--> previous live fiber --sibling--> deleted + // fiber + // + // We can't disconnect `alternate` on nodes that haven't been deleted yet, + // but we can disconnect the `sibling` and `child` pointers. + const previousFiber = parentFiber.alternate; if (previousFiber !== null) { let detachedChild = previousFiber.child; @@ -3613,17 +3624,6 @@ function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void { ); } } - // A fiber was deleted from this parent fiber, but it's still part of - // the previous (alternate) parent fiber's list of children. Because - // children are a linked list, an earlier sibling that's still alive - // will be connected to the deleted fiber via its `alternate`: - // - // live fiber - // --alternate--> previous live fiber - // --sibling--> deleted fiber - // - // We can't disconnect `alternate` on nodes that haven't been deleted - // yet, but we can disconnect the `sibling` and `child` pointers. detachAlternateSiblings(parentFiber); } @@ -3655,11 +3655,32 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { } break; } - // TODO: Disconnect passive effects when a tree is hidden, perhaps after - // a delay. - // case OffscreenComponent: { - // ... - // } + case OffscreenComponent: { + const instance: OffscreenInstance = finishedWork.stateNode; + const nextState: OffscreenState | null = finishedWork.memoizedState; + + const isHidden = nextState !== null; + + if ( + isHidden && + instance.visibility & OffscreenPassiveEffectsConnected && + // For backwards compatibility, don't unmount when a tree suspends. In + // the future we may change this to unmount after a delay. + (finishedWork.return === null || + finishedWork.return.tag !== SuspenseComponent) + ) { + // The effects are currently connected. Disconnect them. + // TODO: Add option or heuristic to delay before disconnecting the + // effects. Then if the tree reappears before the delay has elapsed, we + // can skip toggling the effects entirely. + instance.visibility &= ~OffscreenPassiveEffectsConnected; + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + } else { + recursivelyTraversePassiveUnmountEffects(finishedWork); + } + + break; + } default: { recursivelyTraversePassiveUnmountEffects(finishedWork); break; @@ -3667,6 +3688,70 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { } } +function recursivelyTraverseDisconnectPassiveEffects(parentFiber: Fiber): void { + // Deletions effects can be scheduled on any fiber type. They need to happen + // before the children effects have fired. + const deletions = parentFiber.deletions; + + if ((parentFiber.flags & ChildDeletion) !== NoFlags) { + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const childToDelete = deletions[i]; + // TODO: Convert this to use recursion + nextEffect = childToDelete; + commitPassiveUnmountEffectsInsideOfDeletedTree_begin( + childToDelete, + parentFiber, + ); + } + } + detachAlternateSiblings(parentFiber); + } + + const prevDebugFiber = getCurrentDebugFiberInDEV(); + // TODO: Check PassiveStatic flag + let child = parentFiber.child; + while (child !== null) { + setCurrentDebugFiberInDEV(child); + disconnectPassiveEffect(child); + child = child.sibling; + } + setCurrentDebugFiberInDEV(prevDebugFiber); +} + +function disconnectPassiveEffect(finishedWork: Fiber): void { + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + // TODO: Check PassiveStatic flag + commitHookPassiveUnmountEffects( + finishedWork, + finishedWork.return, + HookPassive, + ); + // When disconnecting passive effects, we fire the effects in the same + // order as during a deletiong: parent before child + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + break; + } + case OffscreenComponent: { + const instance: OffscreenInstance = finishedWork.stateNode; + if (instance.visibility & OffscreenPassiveEffectsConnected) { + instance.visibility &= ~OffscreenPassiveEffectsConnected; + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + } else { + // The effects are already disconnected. + } + break; + } + default: { + recursivelyTraverseDisconnectPassiveEffects(finishedWork); + break; + } + } +} + function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( deletedSubtreeRoot: Fiber, nearestMountedAncestor: Fiber | null, diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index 7658c3aedbdc6..ac9da5a598ef5 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -7,6 +7,7 @@ let Offscreen; let useState; let useLayoutEffect; let useEffect; +let useMemo; let startTransition; describe('ReactOffscreen', () => { @@ -22,6 +23,7 @@ describe('ReactOffscreen', () => { useState = React.useState; useLayoutEffect = React.useLayoutEffect; useEffect = React.useEffect; + useMemo = React.useMemo; startTransition = React.startTransition; }); @@ -939,7 +941,122 @@ describe('ReactOffscreen', () => { }); // @gate enableOffscreen - it("don't defer passive effects when prerendering in a tree whose effects are already connected", async () => { + it('passive effects are connected and disconnected when the visibility changes', async () => { + function Child({step}) { + useEffect(() => { + Scheduler.unstable_yieldValue(`Commit mount [${step}]`); + return () => { + Scheduler.unstable_yieldValue(`Commit unmount [${step}]`); + }; + }, [step]); + return ; + } + + function App({show, step}) { + return ( + + {useMemo( + () => ( + + ), + [step], + )} + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([1, 'Commit mount [1]']); + expect(root).toMatchRenderedOutput(); + + // Hide the tree. This will unmount the effect. + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Commit unmount [1]']); + expect(root).toMatchRenderedOutput(