Skip to content

Commit e582972

Browse files
acdliteacdlite
committed
Add more coverage for nested Offscreen cases
1 parent 47598d0 commit e582972

File tree

3 files changed

+176
-18
lines changed

3 files changed

+176
-18
lines changed

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

+16-9
Original file line numberDiff line numberDiff line change
@@ -1513,15 +1513,22 @@ function completeWork(
15131513
const nextState: OffscreenState | null = workInProgress.memoizedState;
15141514
const nextIsHidden = nextState !== null;
15151515

1516-
if (current !== null) {
1517-
const prevState: OffscreenState | null = current.memoizedState;
1518-
const prevIsHidden = prevState !== null;
1519-
if (
1520-
prevIsHidden !== nextIsHidden &&
1521-
// LegacyHidden doesn't do any hiding — it only pre-renders.
1522-
(!enableLegacyHidden || workInProgress.tag !== LegacyHiddenComponent)
1523-
) {
1524-
workInProgress.flags |= Visibility;
1516+
// Schedule a Visibility effect if the visibility has changed
1517+
if (enableLegacyHidden && workInProgress.tag === LegacyHiddenComponent) {
1518+
// LegacyHidden doesn't do any hiding — it only pre-renders.
1519+
} else {
1520+
if (current !== null) {
1521+
const prevState: OffscreenState | null = current.memoizedState;
1522+
const prevIsHidden = prevState !== null;
1523+
if (prevIsHidden !== nextIsHidden) {
1524+
workInProgress.flags |= Visibility;
1525+
}
1526+
} else {
1527+
// On initial mount, we only need a Visibility effect if the tree
1528+
// is hidden.
1529+
if (nextIsHidden) {
1530+
workInProgress.flags |= Visibility;
1531+
}
15251532
}
15261533
}
15271534

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

+16-9
Original file line numberDiff line numberDiff line change
@@ -1513,15 +1513,22 @@ function completeWork(
15131513
const nextState: OffscreenState | null = workInProgress.memoizedState;
15141514
const nextIsHidden = nextState !== null;
15151515

1516-
if (current !== null) {
1517-
const prevState: OffscreenState | null = current.memoizedState;
1518-
const prevIsHidden = prevState !== null;
1519-
if (
1520-
prevIsHidden !== nextIsHidden &&
1521-
// LegacyHidden doesn't do any hiding — it only pre-renders.
1522-
(!enableLegacyHidden || workInProgress.tag !== LegacyHiddenComponent)
1523-
) {
1524-
workInProgress.flags |= Visibility;
1516+
// Schedule a Visibility effect if the visibility has changed
1517+
if (enableLegacyHidden && workInProgress.tag === LegacyHiddenComponent) {
1518+
// LegacyHidden doesn't do any hiding — it only pre-renders.
1519+
} else {
1520+
if (current !== null) {
1521+
const prevState: OffscreenState | null = current.memoizedState;
1522+
const prevIsHidden = prevState !== null;
1523+
if (prevIsHidden !== nextIsHidden) {
1524+
workInProgress.flags |= Visibility;
1525+
}
1526+
} else {
1527+
// On initial mount, we only need a Visibility effect if the tree
1528+
// is hidden.
1529+
if (nextIsHidden) {
1530+
workInProgress.flags |= Visibility;
1531+
}
15251532
}
15261533
}
15271534

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

+144
Original file line numberDiff line numberDiff line change
@@ -1115,4 +1115,148 @@ describe('ReactOffscreen', () => {
11151115
'Mount More 2',
11161116
]);
11171117
});
1118+
1119+
// @gate enableOffscreen
1120+
it('does not mount effects when prerendering a nested Offscreen boundary', async () => {
1121+
function Child({label}) {
1122+
useEffect(() => {
1123+
Scheduler.unstable_yieldValue('Mount ' + label);
1124+
return () => {
1125+
Scheduler.unstable_yieldValue('Unmount ' + label);
1126+
};
1127+
}, [label]);
1128+
return <Text text={label} />;
1129+
}
1130+
1131+
function App({showOuter, showInner}) {
1132+
return (
1133+
<Offscreen mode={showOuter ? 'visible' : 'hidden'}>
1134+
{useMemo(
1135+
() => (
1136+
<div>
1137+
<Child label="Outer" />
1138+
{showInner ? (
1139+
<Offscreen mode="visible">
1140+
<div>
1141+
<Child label="Inner" />
1142+
</div>
1143+
</Offscreen>
1144+
) : null}
1145+
</div>
1146+
),
1147+
[showInner],
1148+
)}
1149+
</Offscreen>
1150+
);
1151+
}
1152+
1153+
const root = ReactNoop.createRoot();
1154+
1155+
// Prerender the outer contents. No effects should mount.
1156+
await act(async () => {
1157+
root.render(<App showOuter={false} showInner={false} />);
1158+
});
1159+
expect(Scheduler).toHaveYielded(['Outer']);
1160+
expect(root).toMatchRenderedOutput(
1161+
<div hidden={true}>
1162+
<span prop="Outer" />
1163+
</div>,
1164+
);
1165+
1166+
// Prerender the inner contents. No effects should mount.
1167+
await act(async () => {
1168+
root.render(<App showOuter={false} showInner={true} />);
1169+
});
1170+
expect(Scheduler).toHaveYielded(['Outer', 'Inner']);
1171+
expect(root).toMatchRenderedOutput(
1172+
<div hidden={true}>
1173+
<span prop="Outer" />
1174+
<div>
1175+
<span prop="Inner" />
1176+
</div>
1177+
</div>,
1178+
);
1179+
1180+
// Reveal the prerendered tree
1181+
await act(async () => {
1182+
root.render(<App showOuter={true} showInner={true} />);
1183+
});
1184+
// The effects fire, but the tree is not re-rendered because it already
1185+
// prerendered.
1186+
expect(Scheduler).toHaveYielded(['Mount Outer', 'Mount Inner']);
1187+
expect(root).toMatchRenderedOutput(
1188+
<div>
1189+
<span prop="Outer" />
1190+
<div>
1191+
<span prop="Inner" />
1192+
</div>
1193+
</div>,
1194+
);
1195+
});
1196+
1197+
// @gate enableOffscreen
1198+
it('reveal an outer Offscreen boundary without revealing an inner one', async () => {
1199+
function Child({label}) {
1200+
useEffect(() => {
1201+
Scheduler.unstable_yieldValue('Mount ' + label);
1202+
return () => {
1203+
Scheduler.unstable_yieldValue('Unmount ' + label);
1204+
};
1205+
}, [label]);
1206+
return <Text text={label} />;
1207+
}
1208+
1209+
function App({showOuter, showInner}) {
1210+
return (
1211+
<Offscreen mode={showOuter ? 'visible' : 'hidden'}>
1212+
{useMemo(
1213+
() => (
1214+
<div>
1215+
<Child label="Outer" />
1216+
<Offscreen mode={showInner ? 'visible' : 'hidden'}>
1217+
<div>
1218+
<Child label="Inner" />
1219+
</div>
1220+
</Offscreen>
1221+
</div>
1222+
),
1223+
[showInner],
1224+
)}
1225+
</Offscreen>
1226+
);
1227+
}
1228+
1229+
const root = ReactNoop.createRoot();
1230+
1231+
// Prerender the whole tree.
1232+
await act(async () => {
1233+
root.render(<App showOuter={false} showInner={false} />);
1234+
});
1235+
expect(Scheduler).toHaveYielded(['Outer', 'Inner']);
1236+
// Both the inner and the outer tree should be hidden. Hiding the inner tree
1237+
// is arguably redundant, but the advantage of hiding both is that later you
1238+
// can reveal the outer tree without having to examine the inner one.
1239+
expect(root).toMatchRenderedOutput(
1240+
<div hidden={true}>
1241+
<span prop="Outer" />
1242+
<div hidden={true}>
1243+
<span prop="Inner" />
1244+
</div>
1245+
</div>,
1246+
);
1247+
1248+
// Reveal the outer contents. The inner tree remains hidden.
1249+
await act(async () => {
1250+
root.render(<App showOuter={true} showInner={false} />);
1251+
});
1252+
expect(Scheduler).toHaveYielded(['Mount Outer']);
1253+
expect(root).toMatchRenderedOutput(
1254+
<div>
1255+
<span prop="Outer" />
1256+
<div hidden={true}>
1257+
<span prop="Inner" />
1258+
</div>
1259+
</div>,
1260+
);
1261+
});
11181262
});

0 commit comments

Comments
 (0)