Skip to content

Commit

Permalink
Fix for failed Suspense layout semantics (#21694)
Browse files Browse the repository at this point in the history 
Co-authored-by: Dan Abramov <dan.abramov@me.com>
  • Loading branch information
@gaearon
Brian Vaughn and gaearon committed Jun 16, 2021
1 parent bd0a963 commit d0f348d
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 56 deletions.
55 changes: 27 additions & 28 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
55 changes: 27 additions & 28 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
58 changes: 58 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Suspense fallback={<Text text="Loading..." />}>
<TextWithLayout text="Child 1" />
{show && <AsyncText ms={1000} text="Child 2" />}
</Suspense>
);
}

const root = ReactTestRenderer.create(<App />, {
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 {
Expand Down

0 comments on commit d0f348d

Please sign in to comment.