diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 99161bd97f487..a68e489dccc98 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -2121,34 +2121,12 @@ function commitMutationEffectsOnFiber(finishedWork: Fiber, root: FiberRoot) {
case SuspenseComponent: {
const newState: OffscreenState | null = finishedWork.memoizedState;
const isHidden = newState !== null;
- const current = finishedWork.alternate;
- const wasHidden = current !== null && current.memoizedState !== null;
- const offscreenBoundary: Fiber = (finishedWork.child: any);
-
if (isHidden) {
+ const current = finishedWork.alternate;
+ const wasHidden = current !== null && current.memoizedState !== null;
if (!wasHidden) {
+ // TODO: Move to passive phase
markCommitTimeOfFallback();
- if (supportsMutation) {
- hideOrUnhideAllChildren(offscreenBoundary, true);
- }
- if (
- enableSuspenseLayoutEffectSemantics &&
- (offscreenBoundary.mode & ConcurrentMode) !== NoMode
- ) {
- let offscreenChild = offscreenBoundary.child;
- while (offscreenChild !== null) {
- nextEffect = offscreenChild;
- disappearLayoutEffects_begin(offscreenChild);
- offscreenChild = offscreenChild.sibling;
- }
- }
- }
- } else {
- if (wasHidden) {
- if (supportsMutation) {
- hideOrUnhideAllChildren(offscreenBoundary, false);
- }
- // TODO: Move re-appear call here for symmetry?
}
}
break;
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index 8fcdb5920756e..aea3e36ed09d2 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -2121,34 +2121,12 @@ function commitMutationEffectsOnFiber(finishedWork: Fiber, root: FiberRoot) {
case SuspenseComponent: {
const newState: OffscreenState | null = finishedWork.memoizedState;
const isHidden = newState !== null;
- const current = finishedWork.alternate;
- const wasHidden = current !== null && current.memoizedState !== null;
- const offscreenBoundary: Fiber = (finishedWork.child: any);
-
if (isHidden) {
+ const current = finishedWork.alternate;
+ const wasHidden = current !== null && current.memoizedState !== null;
if (!wasHidden) {
+ // TODO: Move to passive phase
markCommitTimeOfFallback();
- if (supportsMutation) {
- hideOrUnhideAllChildren(offscreenBoundary, true);
- }
- if (
- enableSuspenseLayoutEffectSemantics &&
- (offscreenBoundary.mode & ConcurrentMode) !== NoMode
- ) {
- let offscreenChild = offscreenBoundary.child;
- while (offscreenChild !== null) {
- nextEffect = offscreenChild;
- disappearLayoutEffects_begin(offscreenChild);
- offscreenChild = offscreenChild.sibling;
- }
- }
- }
- } else {
- if (wasHidden) {
- if (supportsMutation) {
- hideOrUnhideAllChildren(offscreenBoundary, false);
- }
- // TODO: Move re-appear call here for symmetry?
}
}
break;
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
index c919f15222465..32b9587f26314 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
@@ -324,37 +324,17 @@ if (supportsMutation) {
// If we have a portal child, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
- } else if (node.tag === SuspenseComponent) {
- if ((node.flags & Visibility) !== NoFlags) {
- // Need to toggle the visibility of the primary children.
- const newIsHidden = node.memoizedState !== null;
- if (newIsHidden) {
- const primaryChildParent = node.child;
- if (primaryChildParent !== null) {
- if (primaryChildParent.child !== null) {
- primaryChildParent.child.return = primaryChildParent;
- appendAllChildren(
- parent,
- primaryChildParent,
- true,
- newIsHidden,
- );
- }
- const fallbackChildParent = primaryChildParent.sibling;
- if (fallbackChildParent !== null) {
- fallbackChildParent.return = node;
- node = fallbackChildParent;
- continue;
- }
- }
- }
- }
- if (node.child !== null) {
- // Continue traversing like normal
- node.child.return = node;
- node = node.child;
- continue;
+ } else if (
+ node.tag === OffscreenComponent &&
+ node.memoizedState !== null
+ ) {
+ // The children in this boundary are hidden. Toggle their visibility
+ // before appending.
+ const child = node.child;
+ if (child !== null) {
+ child.return = node;
}
+ appendAllChildren(parent, node, true, true);
} else if (node.child !== null) {
node.child.return = node;
node = node.child;
@@ -409,37 +389,17 @@ if (supportsMutation) {
// If we have a portal child, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
- } else if (node.tag === SuspenseComponent) {
- if ((node.flags & Visibility) !== NoFlags) {
- // Need to toggle the visibility of the primary children.
- const newIsHidden = node.memoizedState !== null;
- if (newIsHidden) {
- const primaryChildParent = node.child;
- if (primaryChildParent !== null) {
- if (primaryChildParent.child !== null) {
- primaryChildParent.child.return = primaryChildParent;
- appendAllChildrenToContainer(
- containerChildSet,
- primaryChildParent,
- true,
- newIsHidden,
- );
- }
- const fallbackChildParent = primaryChildParent.sibling;
- if (fallbackChildParent !== null) {
- fallbackChildParent.return = node;
- node = fallbackChildParent;
- continue;
- }
- }
- }
- }
- if (node.child !== null) {
- // Continue traversing like normal
- node.child.return = node;
- node = node.child;
- continue;
+ } else if (
+ node.tag === OffscreenComponent &&
+ node.memoizedState !== null
+ ) {
+ // The children in this boundary are hidden. Toggle their visibility
+ // before appending.
+ const child = node.child;
+ if (child !== null) {
+ child.return = node;
}
+ appendAllChildrenToContainer(containerChildSet, node, true, true);
} else if (node.child !== null) {
node.child.return = node;
node = node.child;
@@ -1056,7 +1016,21 @@ function completeWork(
prevDidTimeout = prevState !== null;
}
+ // If the suspended state of the boundary changes, we need to schedule
+ // an effect to toggle the subtree's visibility. When we switch from
+ // fallback -> primary, the inner Offscreen fiber schedules this effect
+ // as part of its normal complete phase. But when we switch from
+ // primary -> fallback, the inner Offscreen fiber does not have a complete
+ // phase. So we need to schedule its effect here.
+ //
+ // We also use this flag to connect/disconnect the effects, but the same
+ // logic applies: when re-connecting, the Offscreen fiber's complete
+ // phase will handle scheduling the effect. It's only when the fallback
+ // is active that we have to do anything special.
if (nextDidTimeout && !prevDidTimeout) {
+ const offscreenFiber: Fiber = (workInProgress.child: any);
+ offscreenFiber.flags |= Visibility;
+
// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
@@ -1096,26 +1070,6 @@ function completeWork(
workInProgress.flags |= Update;
}
- if (supportsMutation) {
- if (nextDidTimeout !== prevDidTimeout) {
- // In mutation mode, visibility is toggled by mutating the nearest
- // host nodes whenever they switch from hidden -> visible or vice
- // versa. We don't need to switch when the boundary updates but its
- // visibility hasn't changed.
- workInProgress.flags |= Visibility;
- }
- }
- if (supportsPersistence) {
- if (nextDidTimeout) {
- // In persistent mode, visibility is toggled by cloning the nearest
- // host nodes in the complete phase whenever the boundary is hidden.
- // TODO: The plan is to add a transparent host wrapper (no layout)
- // around the primary children and hide that node. Then we don't need
- // to do the funky cloning business.
- workInProgress.flags |= Visibility;
- }
- }
-
if (
enableSuspenseCallback &&
workInProgress.updateQueue !== null &&
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
index 59d72380dbeeb..60359fd42d8de 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
@@ -324,37 +324,17 @@ if (supportsMutation) {
// If we have a portal child, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
- } else if (node.tag === SuspenseComponent) {
- if ((node.flags & Visibility) !== NoFlags) {
- // Need to toggle the visibility of the primary children.
- const newIsHidden = node.memoizedState !== null;
- if (newIsHidden) {
- const primaryChildParent = node.child;
- if (primaryChildParent !== null) {
- if (primaryChildParent.child !== null) {
- primaryChildParent.child.return = primaryChildParent;
- appendAllChildren(
- parent,
- primaryChildParent,
- true,
- newIsHidden,
- );
- }
- const fallbackChildParent = primaryChildParent.sibling;
- if (fallbackChildParent !== null) {
- fallbackChildParent.return = node;
- node = fallbackChildParent;
- continue;
- }
- }
- }
- }
- if (node.child !== null) {
- // Continue traversing like normal
- node.child.return = node;
- node = node.child;
- continue;
+ } else if (
+ node.tag === OffscreenComponent &&
+ node.memoizedState !== null
+ ) {
+ // The children in this boundary are hidden. Toggle their visibility
+ // before appending.
+ const child = node.child;
+ if (child !== null) {
+ child.return = node;
}
+ appendAllChildren(parent, node, true, true);
} else if (node.child !== null) {
node.child.return = node;
node = node.child;
@@ -409,37 +389,17 @@ if (supportsMutation) {
// If we have a portal child, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
- } else if (node.tag === SuspenseComponent) {
- if ((node.flags & Visibility) !== NoFlags) {
- // Need to toggle the visibility of the primary children.
- const newIsHidden = node.memoizedState !== null;
- if (newIsHidden) {
- const primaryChildParent = node.child;
- if (primaryChildParent !== null) {
- if (primaryChildParent.child !== null) {
- primaryChildParent.child.return = primaryChildParent;
- appendAllChildrenToContainer(
- containerChildSet,
- primaryChildParent,
- true,
- newIsHidden,
- );
- }
- const fallbackChildParent = primaryChildParent.sibling;
- if (fallbackChildParent !== null) {
- fallbackChildParent.return = node;
- node = fallbackChildParent;
- continue;
- }
- }
- }
- }
- if (node.child !== null) {
- // Continue traversing like normal
- node.child.return = node;
- node = node.child;
- continue;
+ } else if (
+ node.tag === OffscreenComponent &&
+ node.memoizedState !== null
+ ) {
+ // The children in this boundary are hidden. Toggle their visibility
+ // before appending.
+ const child = node.child;
+ if (child !== null) {
+ child.return = node;
}
+ appendAllChildrenToContainer(containerChildSet, node, true, true);
} else if (node.child !== null) {
node.child.return = node;
node = node.child;
@@ -1056,7 +1016,21 @@ function completeWork(
prevDidTimeout = prevState !== null;
}
+ // If the suspended state of the boundary changes, we need to schedule
+ // an effect to toggle the subtree's visibility. When we switch from
+ // fallback -> primary, the inner Offscreen fiber schedules this effect
+ // as part of its normal complete phase. But when we switch from
+ // primary -> fallback, the inner Offscreen fiber does not have a complete
+ // phase. So we need to schedule its effect here.
+ //
+ // We also use this flag to connect/disconnect the effects, but the same
+ // logic applies: when re-connecting, the Offscreen fiber's complete
+ // phase will handle scheduling the effect. It's only when the fallback
+ // is active that we have to do anything special.
if (nextDidTimeout && !prevDidTimeout) {
+ const offscreenFiber: Fiber = (workInProgress.child: any);
+ offscreenFiber.flags |= Visibility;
+
// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
@@ -1096,26 +1070,6 @@ function completeWork(
workInProgress.flags |= Update;
}
- if (supportsMutation) {
- if (nextDidTimeout !== prevDidTimeout) {
- // In mutation mode, visibility is toggled by mutating the nearest
- // host nodes whenever they switch from hidden -> visible or vice
- // versa. We don't need to switch when the boundary updates but its
- // visibility hasn't changed.
- workInProgress.flags |= Visibility;
- }
- }
- if (supportsPersistence) {
- if (nextDidTimeout) {
- // In persistent mode, visibility is toggled by cloning the nearest
- // host nodes in the complete phase whenever the boundary is hidden.
- // TODO: The plan is to add a transparent host wrapper (no layout)
- // around the primary children and hide that node. Then we don't need
- // to do the funky cloning business.
- workInProgress.flags |= Visibility;
- }
- }
-
if (
enableSuspenseCallback &&
workInProgress.updateQueue !== null &&
diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
index 327e72fdc9724..2915b1e769f04 100644
--- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
@@ -201,10 +201,14 @@ describe('ReactOffscreen', () => {
});
// No layout effect.
expect(Scheduler).toHaveYielded(['Child']);
- // TODO: Offscreen does not yet hide/unhide children correctly. Until we do,
- // it should only be used inside a host component wrapper whose visibility
- // is toggled simultaneously.
- expect(root).toMatchRenderedOutput();
+ if (gate(flags => flags.persistent)) {
+ expect(root).toMatchRenderedOutput();
+ } else {
+ // TODO: Offscreen does not yet hide/unhide children correctly in mutation
+ // mode. Until we do, it should only be used inside a host component
+ // wrapper whose visibility is toggled simultaneously.
+ expect(root).toMatchRenderedOutput();
+ }
// Unhide the tree. The layout effect is mounted.
await act(async () => {
@@ -251,10 +255,14 @@ describe('ReactOffscreen', () => {
);
});
expect(Scheduler).toHaveYielded(['Unmount layout', 'Child']);
- // TODO: Offscreen does not yet hide/unhide children correctly. Until we do,
- // it should only be used inside a host component wrapper whose visibility
- // is toggled simultaneously.
- expect(root).toMatchRenderedOutput();
+ if (gate(flags => flags.persistent)) {
+ expect(root).toMatchRenderedOutput();
+ } else {
+ // TODO: Offscreen does not yet hide/unhide children correctly in mutation
+ // mode. Until we do, it should only be used inside a host component
+ // wrapper whose visibility is toggled simultaneously.
+ expect(root).toMatchRenderedOutput();
+ }
// Unhide the tree. The layout effect is re-mounted.
await act(async () => {
@@ -291,10 +299,14 @@ describe('ReactOffscreen', () => {
);
});
expect(Scheduler).toHaveYielded(['Child']);
- // TODO: Offscreen does not yet hide/unhide children correctly. Until we do,
- // it should only be used inside a host component wrapper whose visibility
- // is toggled simultaneously.
- expect(root).toMatchRenderedOutput();
+ if (gate(flags => flags.persistent)) {
+ expect(root).toMatchRenderedOutput();
+ } else {
+ // TODO: Offscreen does not yet hide/unhide children correctly in mutation
+ // mode. Until we do, it should only be used inside a host component
+ // wrapper whose visibility is toggled simultaneously.
+ expect(root).toMatchRenderedOutput();
+ }
// Show the tree. The layout effect is mounted.
await act(async () => {
@@ -316,9 +328,13 @@ describe('ReactOffscreen', () => {
);
});
expect(Scheduler).toHaveYielded(['Unmount layout', 'Child']);
- // TODO: Offscreen does not yet hide/unhide children correctly. Until we do,
- // it should only be used inside a host component wrapper whose visibility
- // is toggled simultaneously.
- expect(root).toMatchRenderedOutput();
+ if (gate(flags => flags.persistent)) {
+ expect(root).toMatchRenderedOutput();
+ } else {
+ // TODO: Offscreen does not yet hide/unhide children correctly in mutation
+ // mode. Until we do, it should only be used inside a host component
+ // wrapper whose visibility is toggled simultaneously.
+ expect(root).toMatchRenderedOutput();
+ }
});
});