Skip to content

Commit cdb6b4c

Browse files
author
Brian Vaughn
authored
Only hide outermost host nodes when Offscreen is hidden (#21250)
1 parent 7becb2f commit cdb6b4c

File tree

3 files changed

+154
-34
lines changed

3 files changed

+154
-34
lines changed

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,21 +1025,24 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
10251025
const current = finishedWork.alternate;
10261026
const wasHidden = current !== null && current.memoizedState !== null;
10271027

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

10311031
if (supportsMutation) {
10321032
// We only have the top Fiber that was inserted but we need to recurse down its
10331033
// children to find all the terminal nodes.
10341034
let node: Fiber = finishedWork;
10351035
while (true) {
10361036
if (node.tag === HostComponent) {
1037-
const instance = node.stateNode;
1038-
if (isHidden && hiddenHostSubtreeRoot === null) {
1039-
hiddenHostSubtreeRoot = node;
1040-
hideInstance(instance);
1041-
} else {
1042-
unhideInstance(node.stateNode, node.memoizedProps);
1037+
if (hostSubtreeRoot === null) {
1038+
hostSubtreeRoot = node;
1039+
1040+
const instance = node.stateNode;
1041+
if (isHidden) {
1042+
hideInstance(instance);
1043+
} else {
1044+
unhideInstance(node.stateNode, node.memoizedProps);
1045+
}
10431046
}
10441047

10451048
if (enableSuspenseLayoutEffectSemantics && isModernRoot) {
@@ -1058,11 +1061,13 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
10581061
}
10591062
}
10601063
} else if (node.tag === HostText) {
1061-
const instance = node.stateNode;
1062-
if (isHidden && hiddenHostSubtreeRoot === null) {
1063-
hideTextInstance(instance);
1064-
} else {
1065-
unhideTextInstance(instance, node.memoizedProps);
1064+
if (hostSubtreeRoot === null) {
1065+
const instance = node.stateNode;
1066+
if (isHidden) {
1067+
hideTextInstance(instance);
1068+
} else {
1069+
unhideTextInstance(instance, node.memoizedProps);
1070+
}
10661071
}
10671072
} else if (
10681073
(node.tag === OffscreenComponent ||
@@ -1132,15 +1137,15 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
11321137
return;
11331138
}
11341139

1135-
if (hiddenHostSubtreeRoot === node) {
1136-
hiddenHostSubtreeRoot = null;
1140+
if (hostSubtreeRoot === node) {
1141+
hostSubtreeRoot = null;
11371142
}
11381143

11391144
node = node.return;
11401145
}
11411146

1142-
if (hiddenHostSubtreeRoot === node) {
1143-
hiddenHostSubtreeRoot = null;
1147+
if (hostSubtreeRoot === node) {
1148+
hostSubtreeRoot = null;
11441149
}
11451150

11461151
node.sibling.return = node.return;

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,21 +1025,24 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
10251025
const current = finishedWork.alternate;
10261026
const wasHidden = current !== null && current.memoizedState !== null;
10271027

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

10311031
if (supportsMutation) {
10321032
// We only have the top Fiber that was inserted but we need to recurse down its
10331033
// children to find all the terminal nodes.
10341034
let node: Fiber = finishedWork;
10351035
while (true) {
10361036
if (node.tag === HostComponent) {
1037-
const instance = node.stateNode;
1038-
if (isHidden && hiddenHostSubtreeRoot === null) {
1039-
hiddenHostSubtreeRoot = node;
1040-
hideInstance(instance);
1041-
} else {
1042-
unhideInstance(node.stateNode, node.memoizedProps);
1037+
if (hostSubtreeRoot === null) {
1038+
hostSubtreeRoot = node;
1039+
1040+
const instance = node.stateNode;
1041+
if (isHidden) {
1042+
hideInstance(instance);
1043+
} else {
1044+
unhideInstance(node.stateNode, node.memoizedProps);
1045+
}
10431046
}
10441047

10451048
if (enableSuspenseLayoutEffectSemantics && isModernRoot) {
@@ -1058,11 +1061,13 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
10581061
}
10591062
}
10601063
} else if (node.tag === HostText) {
1061-
const instance = node.stateNode;
1062-
if (isHidden && hiddenHostSubtreeRoot === null) {
1063-
hideTextInstance(instance);
1064-
} else {
1065-
unhideTextInstance(instance, node.memoizedProps);
1064+
if (hostSubtreeRoot === null) {
1065+
const instance = node.stateNode;
1066+
if (isHidden) {
1067+
hideTextInstance(instance);
1068+
} else {
1069+
unhideTextInstance(instance, node.memoizedProps);
1070+
}
10661071
}
10671072
} else if (
10681073
(node.tag === OffscreenComponent ||
@@ -1132,15 +1137,15 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) {
11321137
return;
11331138
}
11341139

1135-
if (hiddenHostSubtreeRoot === node) {
1136-
hiddenHostSubtreeRoot = null;
1140+
if (hostSubtreeRoot === node) {
1141+
hostSubtreeRoot = null;
11371142
}
11381143

11391144
node = node.return;
11401145
}
11411146

1142-
if (hiddenHostSubtreeRoot === node) {
1143-
hiddenHostSubtreeRoot = null;
1147+
if (hostSubtreeRoot === node) {
1148+
hostSubtreeRoot = null;
11441149
}
11451150

11461151
node.sibling.return = node.return;

‎packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,6 +1284,116 @@ describe('ReactSuspenseEffectsSemantics', () => {
12841284
]);
12851285
});
12861286

1287+
// @gate enableSuspenseLayoutEffectSemantics
1288+
// @gate enableCache
1289+
it('should show nested host nodes if multiple boundaries resolve at the same time', async () => {
1290+
function App({innerChildren = null, outerChildren = null}) {
1291+
return (
1292+
<Suspense fallback={<Text text="OuterFallback" />}>
1293+
<Text text="Outer" />
1294+
{outerChildren}
1295+
<Suspense fallback={<Text text="InnerFallback" />}>
1296+
<Text text="Inner" />
1297+
{innerChildren}
1298+
</Suspense>
1299+
</Suspense>
1300+
);
1301+
}
1302+
1303+
// Mount
1304+
await ReactNoop.act(async () => {
1305+
ReactNoop.render(<App />);
1306+
});
1307+
expect(Scheduler).toHaveYielded([
1308+
'Text:Outer render',
1309+
'Text:Inner render',
1310+
'Text:Outer create layout',
1311+
'Text:Inner create layout',
1312+
'Text:Outer create passive',
1313+
'Text:Inner create passive',
1314+
]);
1315+
expect(ReactNoop.getChildren()).toEqual([span('Outer'), span('Inner')]);
1316+
1317+
// Suspend the inner Suspense subtree (only inner effects should be destroyed)
1318+
ReactNoop.act(() => {
1319+
ReactNoop.render(
1320+
<App innerChildren={<AsyncText text="InnerAsync_1" ms={1000} />} />,
1321+
);
1322+
});
1323+
await advanceTimers(1000);
1324+
expect(Scheduler).toHaveYielded([
1325+
'Text:Outer render',
1326+
'Text:Inner render',
1327+
'Suspend:InnerAsync_1',
1328+
'Text:InnerFallback render',
1329+
'Text:Inner destroy layout',
1330+
'Text:InnerFallback create layout',
1331+
]);
1332+
expect(Scheduler).toFlushAndYield(['Text:InnerFallback create passive']);
1333+
expect(ReactNoop.getChildren()).toEqual([
1334+
span('Outer'),
1335+
spanHidden('Inner'),
1336+
span('InnerFallback'),
1337+
]);
1338+
1339+
// Suspend the outer Suspense subtree (outer effects and inner fallback effects should be destroyed)
1340+
// (This check also ensures we don't destroy effects for mounted inner fallback.)
1341+
ReactNoop.act(() => {
1342+
ReactNoop.render(
1343+
<App
1344+
outerChildren={<AsyncText text="OuterAsync_1" ms={1000} />}
1345+
innerChildren={<AsyncText text="InnerAsync_1" ms={1000} />}
1346+
/>,
1347+
);
1348+
});
1349+
await advanceTimers(1000);
1350+
expect(Scheduler).toHaveYielded([
1351+
'Text:Outer render',
1352+
'Suspend:OuterAsync_1',
1353+
'Text:Inner render',
1354+
'Suspend:InnerAsync_1',
1355+
'Text:InnerFallback render',
1356+
'Text:OuterFallback render',
1357+
'Text:Outer destroy layout',
1358+
'Text:InnerFallback destroy layout',
1359+
'Text:OuterFallback create layout',
1360+
]);
1361+
expect(Scheduler).toFlushAndYield(['Text:OuterFallback create passive']);
1362+
expect(ReactNoop.getChildren()).toEqual([
1363+
spanHidden('Outer'),
1364+
spanHidden('Inner'),
1365+
spanHidden('InnerFallback'),
1366+
span('OuterFallback'),
1367+
]);
1368+
1369+
// Resolve both suspended trees.
1370+
await ReactNoop.act(async () => {
1371+
await resolveText('OuterAsync_1');
1372+
await resolveText('InnerAsync_1');
1373+
});
1374+
expect(Scheduler).toHaveYielded([
1375+
'Text:Outer render',
1376+
'AsyncText:OuterAsync_1 render',
1377+
'Text:Inner render',
1378+
'AsyncText:InnerAsync_1 render',
1379+
'Text:OuterFallback destroy layout',
1380+
'Text:Outer create layout',
1381+
'AsyncText:OuterAsync_1 create layout',
1382+
'Text:Inner create layout',
1383+
'AsyncText:InnerAsync_1 create layout',
1384+
'Text:OuterFallback destroy passive',
1385+
'Text:InnerFallback destroy passive',
1386+
'AsyncText:OuterAsync_1 create passive',
1387+
'AsyncText:InnerAsync_1 create passive',
1388+
]);
1389+
expect(ReactNoop.getChildren()).toEqual([
1390+
span('Outer'),
1391+
span('OuterAsync_1'),
1392+
span('Inner'),
1393+
span('InnerAsync_1'),
1394+
]);
1395+
});
1396+
12871397
// @gate enableSuspenseLayoutEffectSemantics
12881398
// @gate enableCache
12891399
it('should be cleaned up inside of a fallback that suspends', async () => {

0 commit comments

Comments
 (0)