Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only hide outermost host nodes when Offscreen is hidden #21250

Merged
merged 1 commit into from
Apr 20, 2021
Merged

Only hide outermost host nodes when Offscreen is hidden #21250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 22 additions & 17 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,21 +1025,24 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
const current = finishedWork.alternate;
const wasHidden = current !== null && current.memoizedState !== null;

// Only hide the top-most host nodes.
let hiddenHostSubtreeRoot = null;
// Only hide or unhide the top-most host nodes.
let hostSubtreeRoot = null;

if (supportsMutation) {
// We only have the top Fiber that was inserted but we need to recurse down its
// children to find all the terminal nodes.
let node: Fiber = finishedWork;
while (true) {
if (node.tag === HostComponent) {
const instance = node.stateNode;
if (isHidden && hiddenHostSubtreeRoot === null) {
hiddenHostSubtreeRoot = node;
hideInstance(instance);
} else {
unhideInstance(node.stateNode, node.memoizedProps);
if (hostSubtreeRoot === null) {
hostSubtreeRoot = node;

const instance = node.stateNode;
if (isHidden) {
hideInstance(instance);
} else {
unhideInstance(node.stateNode, node.memoizedProps);
}
}

if (enableSuspenseLayoutEffectSemantics && isModernRoot) {
Expand All @@ -1058,11 +1061,13 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
}
}
} else if (node.tag === HostText) {
const instance = node.stateNode;
if (isHidden && hiddenHostSubtreeRoot === null) {
hideTextInstance(instance);
} else {
unhideTextInstance(instance, node.memoizedProps);
if (hostSubtreeRoot === null) {
const instance = node.stateNode;
if (isHidden) {
hideTextInstance(instance);
} else {
unhideTextInstance(instance, node.memoizedProps);
}
}
} else if (
(node.tag === OffscreenComponent ||
Expand Down Expand Up @@ -1132,15 +1137,15 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
return;
}

if (hiddenHostSubtreeRoot === node) {
hiddenHostSubtreeRoot = null;
if (hostSubtreeRoot === node) {
hostSubtreeRoot = null;
}

node = node.return;
}

if (hiddenHostSubtreeRoot === node) {
hiddenHostSubtreeRoot = null;
if (hostSubtreeRoot === node) {
hostSubtreeRoot = null;
}

node.sibling.return = node.return;
Expand Down
39 changes: 22 additions & 17 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,21 +1025,24 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
const current = finishedWork.alternate;
const wasHidden = current !== null && current.memoizedState !== null;

// Only hide the top-most host nodes.
let hiddenHostSubtreeRoot = null;
// Only hide or unhide the top-most host nodes.
let hostSubtreeRoot = null;

if (supportsMutation) {
// We only have the top Fiber that was inserted but we need to recurse down its
// children to find all the terminal nodes.
let node: Fiber = finishedWork;
while (true) {
if (node.tag === HostComponent) {
const instance = node.stateNode;
if (isHidden && hiddenHostSubtreeRoot === null) {
hiddenHostSubtreeRoot = node;
hideInstance(instance);
} else {
unhideInstance(node.stateNode, node.memoizedProps);
if (hostSubtreeRoot === null) {
hostSubtreeRoot = node;

const instance = node.stateNode;
if (isHidden) {
hideInstance(instance);
} else {
unhideInstance(node.stateNode, node.memoizedProps);
}
}

if (enableSuspenseLayoutEffectSemantics && isModernRoot) {
Expand All @@ -1058,11 +1061,13 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
}
}
} else if (node.tag === HostText) {
const instance = node.stateNode;
if (isHidden && hiddenHostSubtreeRoot === null) {
hideTextInstance(instance);
} else {
unhideTextInstance(instance, node.memoizedProps);
if (hostSubtreeRoot === null) {
const instance = node.stateNode;
if (isHidden) {
hideTextInstance(instance);
} else {
unhideTextInstance(instance, node.memoizedProps);
}
}
} else if (
(node.tag === OffscreenComponent ||
Expand Down Expand Up @@ -1132,15 +1137,15 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
return;
}

if (hiddenHostSubtreeRoot === node) {
hiddenHostSubtreeRoot = null;
if (hostSubtreeRoot === node) {
hostSubtreeRoot = null;
}

node = node.return;
}

if (hiddenHostSubtreeRoot === node) {
hiddenHostSubtreeRoot = null;
if (hostSubtreeRoot === node) {
hostSubtreeRoot = null;
}

node.sibling.return = node.return;
Expand Down
110 changes: 110 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,116 @@ describe('ReactSuspenseEffectsSemantics', () => {
]);
});

// @gate enableSuspenseLayoutEffectSemantics
// @gate enableCache
it('should show nested host nodes if multiple boundaries resolve at the same time', async () => {
function App({innerChildren = null, outerChildren = null}) {
return (
<Suspense fallback={<Text text="OuterFallback" />}>
<Text text="Outer" />
{outerChildren}
<Suspense fallback={<Text text="InnerFallback" />}>
<Text text="Inner" />
{innerChildren}
</Suspense>
</Suspense>
);
}

// Mount
await ReactNoop.act(async () => {
ReactNoop.render(<App />);
});
expect(Scheduler).toHaveYielded([
'Text:Outer render',
'Text:Inner render',
'Text:Outer create layout',
'Text:Inner create layout',
'Text:Outer create passive',
'Text:Inner create passive',
]);
expect(ReactNoop.getChildren()).toEqual([span('Outer'), span('Inner')]);

// Suspend the inner Suspense subtree (only inner effects should be destroyed)
ReactNoop.act(() => {
ReactNoop.render(
<App innerChildren={<AsyncText text="InnerAsync_1" ms={1000} />} />,
);
});
await advanceTimers(1000);
expect(Scheduler).toHaveYielded([
'Text:Outer render',
'Text:Inner render',
'Suspend:InnerAsync_1',
'Text:InnerFallback render',
'Text:Inner destroy layout',
'Text:InnerFallback create layout',
]);
expect(Scheduler).toFlushAndYield(['Text:InnerFallback create passive']);
expect(ReactNoop.getChildren()).toEqual([
span('Outer'),
spanHidden('Inner'),
span('InnerFallback'),
]);

// Suspend the outer Suspense subtree (outer effects and inner fallback effects should be destroyed)
// (This check also ensures we don't destroy effects for mounted inner fallback.)
ReactNoop.act(() => {
ReactNoop.render(
<App
outerChildren={<AsyncText text="OuterAsync_1" ms={1000} />}
innerChildren={<AsyncText text="InnerAsync_1" ms={1000} />}
/>,
);
});
await advanceTimers(1000);
expect(Scheduler).toHaveYielded([
'Text:Outer render',
'Suspend:OuterAsync_1',
'Text:Inner render',
'Suspend:InnerAsync_1',
'Text:InnerFallback render',
'Text:OuterFallback render',
'Text:Outer destroy layout',
'Text:InnerFallback destroy layout',
'Text:OuterFallback create layout',
]);
expect(Scheduler).toFlushAndYield(['Text:OuterFallback create passive']);
expect(ReactNoop.getChildren()).toEqual([
spanHidden('Outer'),
spanHidden('Inner'),
spanHidden('InnerFallback'),
span('OuterFallback'),
]);

// Resolve both suspended trees.
await ReactNoop.act(async () => {
await resolveText('OuterAsync_1');
await resolveText('InnerAsync_1');
});
expect(Scheduler).toHaveYielded([
'Text:Outer render',
'AsyncText:OuterAsync_1 render',
'Text:Inner render',
'AsyncText:InnerAsync_1 render',
'Text:OuterFallback destroy layout',
'Text:Outer create layout',
'AsyncText:OuterAsync_1 create layout',
'Text:Inner create layout',
'AsyncText:InnerAsync_1 create layout',
'Text:OuterFallback destroy passive',
'Text:InnerFallback destroy passive',
'AsyncText:OuterAsync_1 create passive',
'AsyncText:InnerAsync_1 create passive',
]);
expect(ReactNoop.getChildren()).toEqual([
span('Outer'),
span('OuterAsync_1'),
span('Inner'),
span('InnerAsync_1'),
]);
});

// @gate enableSuspenseLayoutEffectSemantics
// @gate enableCache
it('should be cleaned up inside of a fallback that suspends', async () => {
Expand Down