Skip to content

Commit

Permalink
"Atomic" passive commit effects must always fire
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@acdlite
acdlite committed Jul 21, 2022
1 parent cac65b7 commit 42a484e
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 28 deletions.
121 changes: 107 additions & 14 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -3454,6 +3452,101 @@ function reconnectPassiveEffects(
}
}

function recursivelyTraverseAtomicPassiveEffects(
finishedRoot: FiberRoot,
parentFiber: Fiber,
committedLanes: Lanes,
committedTransitions: Array<Transition> | 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<Transition> | 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);
Expand Down
121 changes: 107 additions & 14 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -3454,6 +3452,101 @@ function reconnectPassiveEffects(
}
}

function recursivelyTraverseAtomicPassiveEffects(
finishedRoot: FiberRoot,
parentFiber: Fiber,
committedLanes: Lanes,
committedTransitions: Array<Transition> | 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<Transition> | 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);
Expand Down
34 changes: 34 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactCache-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ let getCacheForType;
let Scheduler;
let act;
let Suspense;
let Offscreen;
let useCacheRefresh;
let startTransition;
let useState;
Expand All @@ -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;
Expand Down Expand Up @@ -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 (
<Offscreen mode="hidden">
<div>
{prerenderMore ? (
<Cache>
<AsyncText text="More" />
</Cache>
) : null}
</div>
</Offscreen>
);
}

const root = ReactNoop.createRoot();
await act(async () => {
root.render(<App prerenderMore={false} />);
});
expect(Scheduler).toHaveYielded([]);
expect(root).toMatchRenderedOutput(<div hidden={true} />);

seedNextTextCache('More');
await act(async () => {
root.render(<App prerenderMore={true} />);
});
expect(Scheduler).toHaveYielded(['More']);
expect(root).toMatchRenderedOutput(<div hidden={true}>More</div>);
});
});
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ describe('ReactOffscreen', () => {
},
);

// @gate enableOffscreen
it('defer passive effects when prerendering a new Offscreen tree', async () => {
function Child({label}) {
useEffect(() => {
Expand Down Expand Up @@ -937,6 +938,7 @@ describe('ReactOffscreen', () => {
]);
});

// @gate enableOffscreen
it("don't defer passive effects when prerendering in a tree whose effects are already connected", async () => {
function Child({label}) {
useEffect(() => {
Expand Down

0 comments on commit 42a484e

Please sign in to comment.