From e6a705b4834c6215db36382c195ad839d9ab2796 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 21 Jul 2022 00:23:20 -0400 Subject: [PATCH] "Atomic" passive commit effects must always fire There are a few cases where commit phase logic always needs to fire even inside a hidden tree. In general, we should try to design algorithms that don't depend on a commit effect running during prerendering, but there's at least one case where I think it makes sense. The experimental Cache component uses reference counting to keep track of the lifetime of a cache instance. This allows us to expose an AbortSignal object that data frameworks can use to cancel aborted requests. These cache objects are considered alive even inside a prerendered tree. To implement this I added an "atomic" passive effect traversal that runs even when a tree is hidden. (As a follow up, we should add a special subtree flag so that we can skip over nodes that don't have them. There are a number of similar subtree flag optimizations that we have planned, so I'll leave them for a later refactor.) The only other feature that currently depends on this behavior is Transition Tracing. I did not add a test for this because Transition Tracing is still in development and doesn't yet work with Offscreen. --- .../src/ReactFiberCommitWork.new.js | 121 ++++++++++++++++-- .../src/ReactFiberCommitWork.old.js | 121 ++++++++++++++++-- .../src/__tests__/ReactCache-test.js | 34 +++++ .../src/__tests__/ReactOffscreen-test.js | 2 + 4 files changed, 250 insertions(+), 28 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 5bb01e1807836..4f9b4ce42689c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -3181,13 +3181,12 @@ function commitPassiveMountOnFiber( // "Atomic" effects are ones that need to fire on every commit, // even during pre-rendering. An example is updating the reference // count on cache instances. - // TODO: Not yet implemented - // recursivelyTraverseAtomicPassiveEffects( - // finishedRoot, - // finishedWork, - // committedLanes, - // committedTransitions, - // ); + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); } } else { // Legacy Mode: Fire the effects even if the tree is hidden. @@ -3363,13 +3362,12 @@ function reconnectPassiveEffects( // "Atomic" effects are ones that need to fire on every commit, // even during pre-rendering. An example is updating the reference // count on cache instances. - // TODO: Not yet implemented - // recursivelyTraverseAtomicPassiveEffects( - // finishedRoot, - // finishedWork, - // committedLanes, - // committedTransitions, - // ); + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); } } else { // Legacy Mode: Fire the effects even if the tree is hidden. @@ -3454,6 +3452,101 @@ function reconnectPassiveEffects( } } +function recursivelyTraverseAtomicPassiveEffects( + finishedRoot: FiberRoot, + parentFiber: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, +) { + // "Atomic" effects are ones that need to fire on every commit, even during + // pre-rendering. We call this function when traversing a hidden tree whose + // regular effects are currently disconnected. + const prevDebugFiber = getCurrentDebugFiberInDEV(); + // TODO: Add special flag for atomic effects + if (parentFiber.subtreeFlags & PassiveMask) { + let child = parentFiber.child; + while (child !== null) { + setCurrentDebugFiberInDEV(child); + commitAtomicPassiveEffects( + finishedRoot, + child, + committedLanes, + committedTransitions, + ); + child = child.sibling; + } + } + setCurrentDebugFiberInDEV(prevDebugFiber); +} + +function commitAtomicPassiveEffects( + finishedRoot: FiberRoot, + finishedWork: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, +) { + // "Atomic" effects are ones that need to fire on every commit, even during + // pre-rendering. We call this function when traversing a hidden tree whose + // regular effects are currently disconnected. + const flags = finishedWork.flags; + switch (finishedWork.tag) { + case OffscreenComponent: { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + const instance: OffscreenInstance = finishedWork.stateNode; + commitOffscreenPassiveMountEffects(current, finishedWork, instance); + } + break; + } + case CacheComponent: { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + commitCachePassiveMountEffect(current, finishedWork); + } + break; + } + case TracingMarkerComponent: { + if (enableTransitionTracing) { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + commitTracingMarkerPassiveMountEffect(finishedWork); + } + break; + } + // Intentional fallthrough to next branch + } + // eslint-disable-next-line-no-fallthrough + default: { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + break; + } + } +} + export function commitPassiveUnmountEffects(finishedWork: Fiber): void { setCurrentDebugFiberInDEV(finishedWork); commitPassiveUnmountOnFiber(finishedWork); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 97cb9c2808e32..bd16246302de8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -3181,13 +3181,12 @@ function commitPassiveMountOnFiber( // "Atomic" effects are ones that need to fire on every commit, // even during pre-rendering. An example is updating the reference // count on cache instances. - // TODO: Not yet implemented - // recursivelyTraverseAtomicPassiveEffects( - // finishedRoot, - // finishedWork, - // committedLanes, - // committedTransitions, - // ); + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); } } else { // Legacy Mode: Fire the effects even if the tree is hidden. @@ -3363,13 +3362,12 @@ function reconnectPassiveEffects( // "Atomic" effects are ones that need to fire on every commit, // even during pre-rendering. An example is updating the reference // count on cache instances. - // TODO: Not yet implemented - // recursivelyTraverseAtomicPassiveEffects( - // finishedRoot, - // finishedWork, - // committedLanes, - // committedTransitions, - // ); + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); } } else { // Legacy Mode: Fire the effects even if the tree is hidden. @@ -3454,6 +3452,101 @@ function reconnectPassiveEffects( } } +function recursivelyTraverseAtomicPassiveEffects( + finishedRoot: FiberRoot, + parentFiber: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, +) { + // "Atomic" effects are ones that need to fire on every commit, even during + // pre-rendering. We call this function when traversing a hidden tree whose + // regular effects are currently disconnected. + const prevDebugFiber = getCurrentDebugFiberInDEV(); + // TODO: Add special flag for atomic effects + if (parentFiber.subtreeFlags & PassiveMask) { + let child = parentFiber.child; + while (child !== null) { + setCurrentDebugFiberInDEV(child); + commitAtomicPassiveEffects( + finishedRoot, + child, + committedLanes, + committedTransitions, + ); + child = child.sibling; + } + } + setCurrentDebugFiberInDEV(prevDebugFiber); +} + +function commitAtomicPassiveEffects( + finishedRoot: FiberRoot, + finishedWork: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, +) { + // "Atomic" effects are ones that need to fire on every commit, even during + // pre-rendering. We call this function when traversing a hidden tree whose + // regular effects are currently disconnected. + const flags = finishedWork.flags; + switch (finishedWork.tag) { + case OffscreenComponent: { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + const instance: OffscreenInstance = finishedWork.stateNode; + commitOffscreenPassiveMountEffects(current, finishedWork, instance); + } + break; + } + case CacheComponent: { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + commitCachePassiveMountEffect(current, finishedWork); + } + break; + } + case TracingMarkerComponent: { + if (enableTransitionTracing) { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + commitTracingMarkerPassiveMountEffect(finishedWork); + } + break; + } + // Intentional fallthrough to next branch + } + // eslint-disable-next-line-no-fallthrough + default: { + recursivelyTraverseAtomicPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + break; + } + } +} + export function commitPassiveUnmountEffects(finishedWork: Fiber): void { setCurrentDebugFiberInDEV(finishedWork); commitPassiveUnmountOnFiber(finishedWork); diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js index 804486721f9cf..970bf940f27cd 100644 --- a/packages/react-reconciler/src/__tests__/ReactCache-test.js +++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js @@ -6,6 +6,7 @@ let getCacheForType; let Scheduler; let act; let Suspense; +let Offscreen; let useCacheRefresh; let startTransition; let useState; @@ -23,6 +24,7 @@ describe('ReactCache', () => { Scheduler = require('scheduler'); act = require('jest-react').act; Suspense = React.Suspense; + Offscreen = React.unstable_Offscreen; getCacheSignal = React.unstable_getCacheSignal; getCacheForType = React.unstable_getCacheForType; useCacheRefresh = React.unstable_useCacheRefresh; @@ -1590,4 +1592,36 @@ describe('ReactCache', () => { ]); expect(root).toMatchRenderedOutput('Bye!'); }); + + // @gate enableOffscreen + // @gate enableCache + test('prerender a new cache boundary inside an Offscreen tree', async () => { + function App({prerenderMore}) { + return ( + +
+ {prerenderMore ? ( + + + + ) : null} +
+ + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([]); + expect(root).toMatchRenderedOutput(