Skip to content

Commit d1e35c7

Browse files
authored
Don't disappear layout effects unnecessarily (#25660)
Nested Offscreens can run into a case where outer Offscreen is revealed while inner one is hidden in a single commit. This is an edge case that was previously missed. We need to prevent call to disappear layout effects. When we go from state: ```jsx <Offscreen mode={'hidden'}> // outer offscreen <Offscreen mode={'visible'}> // inner offscreen {children} </Offscreen> </Offscreen> ``` To following. Notice that visibility of each offscreen flips. ```jsx <Offscreen mode={'visible'}> // outer offscreen <Offscreen mode={'hidden'}> // inner offscreen {children} </Offscreen> </Offscreen> ``` Inner offscreen must not call `recursivelyTraverseDisappearLayoutEffects`. Check unit tests for an example of this.
1 parent 1e3e30d commit d1e35c7

File tree

3 files changed

+75
-31
lines changed

3 files changed

+75
-31
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,12 +2880,13 @@ function commitMutationEffectsOnFiber(
28802880

28812881
if (isHidden) {
28822882
const isUpdate = current !== null;
2883-
const isAncestorOffscreenHidden = offscreenSubtreeIsHidden;
2883+
const wasHiddenByAncestorOffscreen =
2884+
offscreenSubtreeIsHidden || offscreenSubtreeWasHidden;
28842885
// Only trigger disapper layout effects if:
28852886
// - This is an update, not first mount.
28862887
// - This Offscreen was not hidden before.
2887-
// - No ancestor Offscreen is hidden.
2888-
if (isUpdate && !wasHidden && !isAncestorOffscreenHidden) {
2888+
// - Ancestor Offscreen was not hidden in previous commit.
2889+
if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
28892890
if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
28902891
// Disappear the layout effects of all the children
28912892
recursivelyTraverseDisappearLayoutEffects(finishedWork);

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,12 +2880,13 @@ function commitMutationEffectsOnFiber(
28802880

28812881
if (isHidden) {
28822882
const isUpdate = current !== null;
2883-
const isAncestorOffscreenHidden = offscreenSubtreeIsHidden;
2883+
const wasHiddenByAncestorOffscreen =
2884+
offscreenSubtreeIsHidden || offscreenSubtreeWasHidden;
28842885
// Only trigger disapper layout effects if:
28852886
// - This is an update, not first mount.
28862887
// - This Offscreen was not hidden before.
2887-
// - No ancestor Offscreen is hidden.
2888-
if (isUpdate && !wasHidden && !isAncestorOffscreenHidden) {
2888+
// - Ancestor Offscreen was not hidden in previous commit.
2889+
if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
28892890
if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
28902891
// Disappear the layout effects of all the children
28912892
recursivelyTraverseDisappearLayoutEffects(finishedWork);

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

Lines changed: 67 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ describe('ReactOffscreen', () => {
275275
// goes from visible to hidden in synchronous update.
276276
class ClassComponent extends React.Component {
277277
render() {
278-
return <Text text="Child" />;
278+
return <Text text="child" />;
279279
}
280280

281281
componentWillUnmount() {
@@ -287,47 +287,89 @@ describe('ReactOffscreen', () => {
287287
}
288288
}
289289

290-
let _setIsVisible;
291-
let isFirstRender = true;
290+
const root = ReactNoop.createRoot();
291+
await act(async () => {
292+
// Outer and inner offscreen are hidden.
293+
root.render(
294+
<Offscreen mode={'hidden'}>
295+
<Offscreen mode={'hidden'}>
296+
<ClassComponent />
297+
</Offscreen>
298+
</Offscreen>,
299+
);
300+
});
292301

293-
function App() {
294-
const [isVisible, setIsVisible] = React.useState(true);
295-
_setIsVisible = setIsVisible;
296-
if (isFirstRender === true) {
297-
setIsVisible(false);
298-
isFirstRender = false;
299-
}
302+
expect(Scheduler).toHaveYielded(['child']);
303+
expect(root).toMatchRenderedOutput(<span hidden={true} prop="child" />);
300304

301-
return (
302-
<Offscreen mode="hidden">
303-
<Offscreen mode={isVisible ? 'visible' : 'hidden'}>
305+
await act(async () => {
306+
// Inner offscreen is visible.
307+
root.render(
308+
<Offscreen mode={'hidden'}>
309+
<Offscreen mode={'visible'}>
304310
<ClassComponent />
305311
</Offscreen>
306-
</Offscreen>
312+
</Offscreen>,
307313
);
308-
}
314+
});
315+
316+
expect(Scheduler).toHaveYielded(['child']);
317+
expect(root).toMatchRenderedOutput(<span hidden={true} prop="child" />);
309318

310-
const root = ReactNoop.createRoot();
311319
await act(async () => {
312-
root.render(<App />);
320+
// Inner offscreen is hidden.
321+
root.render(
322+
<Offscreen mode={'hidden'}>
323+
<Offscreen mode={'hidden'}>
324+
<ClassComponent />
325+
</Offscreen>
326+
</Offscreen>,
327+
);
313328
});
314329

315-
expect(Scheduler).toHaveYielded(['Child']);
316-
expect(root).toMatchRenderedOutput(<span hidden={true} prop="Child" />);
330+
expect(Scheduler).toHaveYielded(['child']);
331+
expect(root).toMatchRenderedOutput(<span hidden={true} prop="child" />);
317332

318333
await act(async () => {
319-
_setIsVisible(true);
334+
// Inner offscreen is visible.
335+
root.render(
336+
<Offscreen mode={'hidden'}>
337+
<Offscreen mode={'visible'}>
338+
<ClassComponent />
339+
</Offscreen>
340+
</Offscreen>,
341+
);
320342
});
321343

322-
expect(Scheduler).toHaveYielded(['Child']);
323-
expect(root).toMatchRenderedOutput(<span hidden={true} prop="Child" />);
344+
Scheduler.unstable_clearYields();
345+
346+
await act(async () => {
347+
// Outer offscreen is visible.
348+
// Inner offscreen is hidden.
349+
root.render(
350+
<Offscreen mode={'visible'}>
351+
<Offscreen mode={'hidden'}>
352+
<ClassComponent />
353+
</Offscreen>
354+
</Offscreen>,
355+
);
356+
});
357+
358+
expect(Scheduler).toHaveYielded(['child']);
324359

325360
await act(async () => {
326-
_setIsVisible(false);
361+
// Outer offscreen is hidden.
362+
// Inner offscreen is visible.
363+
root.render(
364+
<Offscreen mode={'hidden'}>
365+
<Offscreen mode={'visible'}>
366+
<ClassComponent />
367+
</Offscreen>
368+
</Offscreen>,
369+
);
327370
});
328371

329-
expect(Scheduler).toHaveYielded(['Child']);
330-
expect(root).toMatchRenderedOutput(<span hidden={true} prop="Child" />);
372+
expect(Scheduler).toHaveYielded(['child']);
331373
});
332374

333375
// @gate enableOffscreen

0 commit comments

Comments
 (0)