diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index e31ed8577938..60158f1f4aaa 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -2210,36 +2210,35 @@ function commitLayoutEffects_begin( commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); continue; } else { - if ((fiber.subtreeFlags & LayoutMask) !== NoFlags) { - const current = fiber.alternate; - const wasHidden = current !== null && current.memoizedState !== null; - const newOffscreenSubtreeWasHidden = - wasHidden || offscreenSubtreeWasHidden; - const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; - const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; - - // Traverse the Offscreen subtree with the current Offscreen as the root. - offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden; - offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden; - let child = firstChild; - while (child !== null) { - nextEffect = child; - commitLayoutEffects_begin( - child, // New root; bubble back up to here and stop. - root, - committedLanes, - ); - child = child.sibling; - } + // TODO (Offscreen) Also check: subtreeFlags & LayoutMask + const current = fiber.alternate; + const wasHidden = current !== null && current.memoizedState !== null; + const newOffscreenSubtreeWasHidden = + wasHidden || offscreenSubtreeWasHidden; + const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; + const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; + + // Traverse the Offscreen subtree with the current Offscreen as the root. + offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden; + offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden; + let child = firstChild; + while (child !== null) { + nextEffect = child; + commitLayoutEffects_begin( + child, // New root; bubble back up to here and stop. + root, + committedLanes, + ); + child = child.sibling; + } - // Restore Offscreen state and resume in our-progress traversal. - nextEffect = fiber; - offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; - offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; - commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); + // Restore Offscreen state and resume in our-progress traversal. + nextEffect = fiber; + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; + offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; + commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); - continue; - } + continue; } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 4dc06f6f4f7d..c741023f22bf 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -2210,36 +2210,35 @@ function commitLayoutEffects_begin( commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); continue; } else { - if ((fiber.subtreeFlags & LayoutMask) !== NoFlags) { - const current = fiber.alternate; - const wasHidden = current !== null && current.memoizedState !== null; - const newOffscreenSubtreeWasHidden = - wasHidden || offscreenSubtreeWasHidden; - const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; - const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; - - // Traverse the Offscreen subtree with the current Offscreen as the root. - offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden; - offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden; - let child = firstChild; - while (child !== null) { - nextEffect = child; - commitLayoutEffects_begin( - child, // New root; bubble back up to here and stop. - root, - committedLanes, - ); - child = child.sibling; - } + // TODO (Offscreen) Also check: subtreeFlags & LayoutMask + const current = fiber.alternate; + const wasHidden = current !== null && current.memoizedState !== null; + const newOffscreenSubtreeWasHidden = + wasHidden || offscreenSubtreeWasHidden; + const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; + const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; + + // Traverse the Offscreen subtree with the current Offscreen as the root. + offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden; + offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden; + let child = firstChild; + while (child !== null) { + nextEffect = child; + commitLayoutEffects_begin( + child, // New root; bubble back up to here and stop. + root, + committedLanes, + ); + child = child.sibling; + } - // Restore Offscreen state and resume in our-progress traversal. - nextEffect = fiber; - offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; - offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; - commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); + // Restore Offscreen state and resume in our-progress traversal. + nextEffect = fiber; + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; + offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; + commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); - continue; - } + continue; } } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index a5db0b503612..2bfec1aa583a 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -663,6 +663,64 @@ describe('ReactSuspense', () => { expect(root).toMatchRenderedOutput('new value'); }); + // @gate enableSuspenseLayoutEffectSemantics + it('re-fires layout effects when re-showing Suspense', () => { + function TextWithLayout(props) { + Scheduler.unstable_yieldValue(props.text); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('create layout'); + return () => { + Scheduler.unstable_yieldValue('destroy layout'); + }; + }, []); + return props.text; + } + + let _setShow; + function App(props) { + const [show, setShow] = React.useState(false); + _setShow = setShow; + return ( + }> + + {show && } + + ); + } + + const root = ReactTestRenderer.create(, { + unstable_isConcurrent: true, + }); + + expect(Scheduler).toFlushAndYield(['Child 1', 'create layout']); + expect(root).toMatchRenderedOutput('Child 1'); + + ReactTestRenderer.act(() => { + _setShow(true); + }); + expect(Scheduler).toHaveYielded( + // DEV behavior differs from prod + // In DEV sometimes the work loop sync-flushes the commit + // where as in production, it schedules it behind a timeout. + // See shouldForceFlushFallbacksInDEV() usage + __DEV__ + ? ['Child 1', 'Suspend! [Child 2]', 'Loading...', 'destroy layout'] + : ['Child 1', 'Suspend! [Child 2]', 'Loading...'], + ); + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded( + // DEV behavior differs from prod + // In DEV sometimes the work loop sync-flushes the commit + // where as in production, it schedules it behind a timeout. + // See shouldForceFlushFallbacksInDEV() usage + __DEV__ + ? ['Promise resolved [Child 2]'] + : ['destroy layout', 'Promise resolved [Child 2]'], + ); + expect(Scheduler).toFlushAndYield(['Child 1', 'Child 2', 'create layout']); + expect(root).toMatchRenderedOutput(['Child 1', 'Child 2'].join('')); + }); + describe('outside concurrent mode', () => { it('a mounted class component can suspend without losing state', () => { class TextWithLifecycle extends React.Component {