Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't disappear layout effects unnecessarily (#25660)

Browse Source 
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.
This commit is contained in:
Samuel Susla
2022-12-03 11:26:06 -05:00
committed by Rick Hanlon
parent 0287a85577
commit 864bc2be64
3 changed files with 80 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+4 -3
View File
@@ -2880,12 +2880,13 @@ function commitMutationEffectsOnFiber(
if (isHidden) {
const isUpdate = current !== null;
const isAncestorOffscreenHidden = offscreenSubtreeIsHidden;
const wasHiddenByAncestorOffscreen =
offscreenSubtreeIsHidden || offscreenSubtreeWasHidden;
// Only trigger disapper layout effects if:
// - This is an update, not first mount.
// - This Offscreen was not hidden before.
// - No ancestor Offscreen is hidden.
if (isUpdate && !wasHidden && !isAncestorOffscreenHidden) {
// - Ancestor Offscreen was not hidden in previous commit.
if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
// Disappear the layout effects of all the children
recursivelyTraverseDisappearLayoutEffects(finishedWork);
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+4 -3
View File
@@ -2880,12 +2880,13 @@ function commitMutationEffectsOnFiber(
if (isHidden) {
const isUpdate = current !== null;
const isAncestorOffscreenHidden = offscreenSubtreeIsHidden;
const wasHiddenByAncestorOffscreen =
offscreenSubtreeIsHidden || offscreenSubtreeWasHidden;
// Only trigger disapper layout effects if:
// - This is an update, not first mount.
// - This Offscreen was not hidden before.
// - No ancestor Offscreen is hidden.
if (isUpdate && !wasHidden && !isAncestorOffscreenHidden) {
// - Ancestor Offscreen was not hidden in previous commit.
if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
// Disappear the layout effects of all the children
recursivelyTraverseDisappearLayoutEffects(finishedWork);
packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
Vendored
+72 -30
View File
@@ -275,7 +275,7 @@ describe('ReactOffscreen', () => {
// goes from visible to hidden in synchronous update.
class ClassComponent extends React.Component {
render() {
return <Text text="Child" />;
return <Text text="child" />;
}
componentWillUnmount() {
@@ -287,47 +287,89 @@ describe('ReactOffscreen', () => {
}
}
let _setIsVisible;
let isFirstRender = true;
function App() {
const [isVisible, setIsVisible] = React.useState(true);
_setIsVisible = setIsVisible;
if (isFirstRender === true) {
setIsVisible(false);
isFirstRender = false;
}
return (
<Offscreen mode="hidden">
<Offscreen mode={isVisible ? 'visible' : 'hidden'}>
<ClassComponent />
</Offscreen>
</Offscreen>
);
}
const root = ReactNoop.createRoot();
await act(async () => {
root.render(<App />);
// Outer and inner offscreen are hidden.
root.render(
<Offscreen mode={'hidden'}>
<Offscreen mode={'hidden'}>
<ClassComponent />
</Offscreen>
</Offscreen>,
);
});
expect(Scheduler).toHaveYielded(['Child']);
expect(root).toMatchRenderedOutput(<span hidden={true} prop="Child" />);
expect(Scheduler).toHaveYielded(['child']);
expect(root).toMatchRenderedOutput(<span hidden={true} prop="child" />);
await act(async () => {
_setIsVisible(true);
// Inner offscreen is visible.
root.render(
<Offscreen mode={'hidden'}>
<Offscreen mode={'visible'}>
<ClassComponent />
</Offscreen>
</Offscreen>,
);
});
expect(Scheduler).toHaveYielded(['Child']);
expect(root).toMatchRenderedOutput(<span hidden={true} prop="Child" />);
expect(Scheduler).toHaveYielded(['child']);
expect(root).toMatchRenderedOutput(<span hidden={true} prop="child" />);
await act(async () => {
_setIsVisible(false);
// Inner offscreen is hidden.
root.render(
<Offscreen mode={'hidden'}>
<Offscreen mode={'hidden'}>
<ClassComponent />
</Offscreen>
</Offscreen>,
);
});
expect(Scheduler).toHaveYielded(['Child']);
expect(root).toMatchRenderedOutput(<span hidden={true} prop="Child" />);
expect(Scheduler).toHaveYielded(['child']);
expect(root).toMatchRenderedOutput(<span hidden={true} prop="child" />);
await act(async () => {
// Inner offscreen is visible.
root.render(
<Offscreen mode={'hidden'}>
<Offscreen mode={'visible'}>
<ClassComponent />
</Offscreen>
</Offscreen>,
);
});
Scheduler.unstable_clearYields();
await act(async () => {
// Outer offscreen is visible.
// Inner offscreen is hidden.
root.render(
<Offscreen mode={'visible'}>
<Offscreen mode={'hidden'}>
<ClassComponent />
</Offscreen>
</Offscreen>,
);
});
expect(Scheduler).toHaveYielded(['child']);
await act(async () => {
// Outer offscreen is hidden.
// Inner offscreen is visible.
root.render(
<Offscreen mode={'hidden'}>
<Offscreen mode={'visible'}>
<ClassComponent />
</Offscreen>
</Offscreen>,
);
});
expect(Scheduler).toHaveYielded(['child']);
});
// @gate enableOffscreen