Skip to content

Commit d0f348d

Browse files
Brian Vaughngaearon
Brian Vaughn
and
gaearon
authored
Fix for failed Suspense layout semantics (#21694)
Co-authored-by: Dan Abramov <[email protected]>
1 parent bd0a963 commit d0f348d

File tree

3 files changed

+112
-56
lines changed

3 files changed

+112
-56
lines changed

‎packages/react-reconciler/src/ReactFiberCommitWork.new.js‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,36 +2210,35 @@ function commitLayoutEffects_begin(
22102210
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
22112211
continue;
22122212
} else {
2213-
if ((fiber.subtreeFlags & LayoutMask) !== NoFlags) {
2214-
const current = fiber.alternate;
2215-
const wasHidden = current !== null && current.memoizedState !== null;
2216-
const newOffscreenSubtreeWasHidden =
2217-
wasHidden || offscreenSubtreeWasHidden;
2218-
const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
2219-
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2220-
2221-
// Traverse the Offscreen subtree with the current Offscreen as the root.
2222-
offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden;
2223-
offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
2224-
let child = firstChild;
2225-
while (child !== null) {
2226-
nextEffect = child;
2227-
commitLayoutEffects_begin(
2228-
child, // New root; bubble back up to here and stop.
2229-
root,
2230-
committedLanes,
2231-
);
2232-
child = child.sibling;
2233-
}
2213+
// TODO (Offscreen) Also check: subtreeFlags & LayoutMask
2214+
const current = fiber.alternate;
2215+
const wasHidden = current !== null && current.memoizedState !== null;
2216+
const newOffscreenSubtreeWasHidden =
2217+
wasHidden || offscreenSubtreeWasHidden;
2218+
const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
2219+
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2220+
2221+
// Traverse the Offscreen subtree with the current Offscreen as the root.
2222+
offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden;
2223+
offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
2224+
let child = firstChild;
2225+
while (child !== null) {
2226+
nextEffect = child;
2227+
commitLayoutEffects_begin(
2228+
child, // New root; bubble back up to here and stop.
2229+
root,
2230+
committedLanes,
2231+
);
2232+
child = child.sibling;
2233+
}
22342234

2235-
// Restore Offscreen state and resume in our-progress traversal.
2236-
nextEffect = fiber;
2237-
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
2238-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2239-
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
2235+
// Restore Offscreen state and resume in our-progress traversal.
2236+
nextEffect = fiber;
2237+
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
2238+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2239+
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
22402240

2241-
continue;
2242-
}
2241+
continue;
22432242
}
22442243
}
22452244

‎packages/react-reconciler/src/ReactFiberCommitWork.old.js‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,36 +2210,35 @@ function commitLayoutEffects_begin(
22102210
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
22112211
continue;
22122212
} else {
2213-
if ((fiber.subtreeFlags & LayoutMask) !== NoFlags) {
2214-
const current = fiber.alternate;
2215-
const wasHidden = current !== null && current.memoizedState !== null;
2216-
const newOffscreenSubtreeWasHidden =
2217-
wasHidden || offscreenSubtreeWasHidden;
2218-
const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
2219-
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2220-
2221-
// Traverse the Offscreen subtree with the current Offscreen as the root.
2222-
offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden;
2223-
offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
2224-
let child = firstChild;
2225-
while (child !== null) {
2226-
nextEffect = child;
2227-
commitLayoutEffects_begin(
2228-
child, // New root; bubble back up to here and stop.
2229-
root,
2230-
committedLanes,
2231-
);
2232-
child = child.sibling;
2233-
}
2213+
// TODO (Offscreen) Also check: subtreeFlags & LayoutMask
2214+
const current = fiber.alternate;
2215+
const wasHidden = current !== null && current.memoizedState !== null;
2216+
const newOffscreenSubtreeWasHidden =
2217+
wasHidden || offscreenSubtreeWasHidden;
2218+
const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
2219+
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2220+
2221+
// Traverse the Offscreen subtree with the current Offscreen as the root.
2222+
offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden;
2223+
offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
2224+
let child = firstChild;
2225+
while (child !== null) {
2226+
nextEffect = child;
2227+
commitLayoutEffects_begin(
2228+
child, // New root; bubble back up to here and stop.
2229+
root,
2230+
committedLanes,
2231+
);
2232+
child = child.sibling;
2233+
}
22342234

2235-
// Restore Offscreen state and resume in our-progress traversal.
2236-
nextEffect = fiber;
2237-
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
2238-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2239-
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
2235+
// Restore Offscreen state and resume in our-progress traversal.
2236+
nextEffect = fiber;
2237+
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
2238+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2239+
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
22402240

2241-
continue;
2242-
}
2241+
continue;
22432242
}
22442243
}
22452244

‎packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,64 @@ describe('ReactSuspense', () => {
663663
expect(root).toMatchRenderedOutput('new value');
664664
});
665665

666+
// @gate enableSuspenseLayoutEffectSemantics
667+
it('re-fires layout effects when re-showing Suspense', () => {
668+
function TextWithLayout(props) {
669+
Scheduler.unstable_yieldValue(props.text);
670+
React.useLayoutEffect(() => {
671+
Scheduler.unstable_yieldValue('create layout');
672+
return () => {
673+
Scheduler.unstable_yieldValue('destroy layout');
674+
};
675+
}, []);
676+
return props.text;
677+
}
678+
679+
let _setShow;
680+
function App(props) {
681+
const [show, setShow] = React.useState(false);
682+
_setShow = setShow;
683+
return (
684+
<Suspense fallback={<Text text="Loading..." />}>
685+
<TextWithLayout text="Child 1" />
686+
{show && <AsyncText ms={1000} text="Child 2" />}
687+
</Suspense>
688+
);
689+
}
690+
691+
const root = ReactTestRenderer.create(<App />, {
692+
unstable_isConcurrent: true,
693+
});
694+
695+
expect(Scheduler).toFlushAndYield(['Child 1', 'create layout']);
696+
expect(root).toMatchRenderedOutput('Child 1');
697+
698+
ReactTestRenderer.act(() => {
699+
_setShow(true);
700+
});
701+
expect(Scheduler).toHaveYielded(
702+
// DEV behavior differs from prod
703+
// In DEV sometimes the work loop sync-flushes the commit
704+
// where as in production, it schedules it behind a timeout.
705+
// See shouldForceFlushFallbacksInDEV() usage
706+
__DEV__
707+
? ['Child 1', 'Suspend! [Child 2]', 'Loading...', 'destroy layout']
708+
: ['Child 1', 'Suspend! [Child 2]', 'Loading...'],
709+
);
710+
jest.advanceTimersByTime(1000);
711+
expect(Scheduler).toHaveYielded(
712+
// DEV behavior differs from prod
713+
// In DEV sometimes the work loop sync-flushes the commit
714+
// where as in production, it schedules it behind a timeout.
715+
// See shouldForceFlushFallbacksInDEV() usage
716+
__DEV__
717+
? ['Promise resolved [Child 2]']
718+
: ['destroy layout', 'Promise resolved [Child 2]'],
719+
);
720+
expect(Scheduler).toFlushAndYield(['Child 1', 'Child 2', 'create layout']);
721+
expect(root).toMatchRenderedOutput(['Child 1', 'Child 2'].join(''));
722+
});
723+
666724
describe('outside concurrent mode', () => {
667725
it('a mounted class component can suspend without losing state', () => {
668726
class TextWithLifecycle extends React.Component {

0 commit comments

Comments
 (0)