Skip to content

Commit ab075a2

Browse files
authored
Do not unmount layout effects on initial Offscreen mount (#25592)
`wasHidden` is evaluted to false if `current` is null. This means Offscreen has never been shown but this code assumes it is going from 'visible' to 'hidden' and unmounts layout effects. To fix this, only unmount layout effects if `current` is not null. I'm not able to repro this problem or write unit test for it. I see this crash bug in production test. The problem with repro is that if Offscreen starts as hidden, it's render is deferred and current is no longer null.
1 parent 765805b commit ab075a2

File tree

3 files changed

+42
-2
lines changed

3 files changed

+42
-2
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -2880,7 +2880,8 @@ function commitMutationEffectsOnFiber(
28802880
}
28812881

28822882
if (isHidden) {
2883-
if (!wasHidden) {
2883+
// Check if this is an update, and the tree was previously visible.
2884+
if (current !== null && !wasHidden) {
28842885
if ((offscreenBoundary.mode & ConcurrentMode) !== NoMode) {
28852886
// Disappear the layout effects of all the children
28862887
recursivelyTraverseDisappearLayoutEffects(offscreenBoundary);

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -2880,7 +2880,8 @@ function commitMutationEffectsOnFiber(
28802880
}
28812881

28822882
if (isHidden) {
2883-
if (!wasHidden) {
2883+
// Check if this is an update, and the tree was previously visible.
2884+
if (current !== null && !wasHidden) {
28842885
if ((offscreenBoundary.mode & ConcurrentMode) !== NoMode) {
28852886
// Disappear the layout effects of all the children
28862887
recursivelyTraverseDisappearLayoutEffects(offscreenBoundary);

packages/react-reconciler/src/__tests__/ReactOffscreen-test.js

+38
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,44 @@ describe('ReactOffscreen', () => {
268268
expect(root).toMatchRenderedOutput(<span prop="Child" />);
269269
});
270270

271+
// @gate enableOffscreen
272+
it('nested offscreen does not call componentWillUnmount when hidden', async () => {
273+
// This is a bug that appeared during production test of <unstable_Offscreen />.
274+
// It is a very specific scenario with nested Offscreens. The inner offscreen
275+
// goes from visible to hidden in synchronous update.
276+
class ClassComponent extends React.Component {
277+
render() {
278+
return <Text text="Child" />;
279+
}
280+
281+
componentWillUnmount() {
282+
Scheduler.unstable_yieldValue('componentWillUnmount');
283+
}
284+
}
285+
286+
function App() {
287+
const [isVisible, setIsVisible] = React.useState(true);
288+
289+
if (isVisible === true) {
290+
setIsVisible(false);
291+
}
292+
293+
return (
294+
<Offscreen mode="hidden">
295+
<Offscreen mode={isVisible ? 'visible' : 'hidden'}>
296+
<ClassComponent />
297+
</Offscreen>
298+
</Offscreen>
299+
);
300+
}
301+
302+
const root = ReactNoop.createRoot();
303+
await act(async () => {
304+
root.render(<App />);
305+
});
306+
expect(Scheduler).toHaveYielded(['Child']);
307+
});
308+
271309
// @gate enableOffscreen
272310
it('mounts/unmounts layout effects when visibility changes (starting hidden)', async () => {
273311
function Child({text}) {

0 commit comments

Comments
 (0)