Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 1d68bce

Browse files
authored
[Fiber] Don't unhide a node if a direct parent offscreen is still hidden (#34821)
If an inner Offscreen commits an unhide, but an outer Offscreen is still hidden but they're controlling the same DOM node then we shouldn't unhide the DOM node yet. This keeps track of whether we're directly inside a hidden offscreen. It might be better to just do the tree search instead of keeping the stack state since it's a rare case. Although this hide/unhide path does trigger a lot of times even when there's no change. This was technically a bug with Suspense too but it doesn't appear because a suspended Suspense boundary never commits its partial state. If it did, it would trigger this same path. But it can happen with an outer Activity and inner Suspense.
1 parent ead9218 commit 1d68bce

3 files changed

Lines changed: 184 additions & 3 deletions

File tree

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ import type {Flags} from './ReactFiberFlags';
292292
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor.
293293
let offscreenSubtreeIsHidden: boolean=false;
294294
let offscreenSubtreeWasHidden: boolean=false;
295+
// Track whether there's a hidden offscreen above with no HostComponent between. If so,
296+
// it overrides the hiddenness of the HostComponent below.
297+
let offscreenDirectParentIsHidden: boolean=false;
295298

296299
// Used to track if a form needs to be reset at the end of the mutation phase.
297300
letneedsFormReset=false;
@@ -2141,8 +2144,14 @@ function commitMutationEffectsOnFiber(
21412144
// Fall through
21422145
}
21432146
caseHostComponent: {
2147+
// We've hit a host component, so it's no longer a direct parent.
2148+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
2149+
offscreenDirectParentIsHidden=false;
2150+
21442151
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
21452152

2153+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
2154+
21462155
commitReconciliationEffects(finishedWork,lanes);
21472156

21482157
if(flags&Ref){
@@ -2422,10 +2431,14 @@ function commitMutationEffectsOnFiber(
24222431
// effects again.
24232432
constprevOffscreenSubtreeIsHidden=offscreenSubtreeIsHidden;
24242433
constprevOffscreenSubtreeWasHidden=offscreenSubtreeWasHidden;
2434+
constprevOffscreenDirectParentIsHidden=offscreenDirectParentIsHidden;
24252435
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden||isHidden;
2436+
offscreenDirectParentIsHidden=
2437+
prevOffscreenDirectParentIsHidden||isHidden;
24262438
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden||wasHidden;
24272439
recursivelyTraverseMutationEffects(root,finishedWork,lanes);
24282440
offscreenSubtreeWasHidden=prevOffscreenSubtreeWasHidden;
2441+
offscreenDirectParentIsHidden=prevOffscreenDirectParentIsHidden;
24292442
offscreenSubtreeIsHidden=prevOffscreenSubtreeIsHidden;
24302443

24312444
if(
@@ -2504,9 +2517,10 @@ function commitMutationEffectsOnFiber(
25042517
}
25052518

25062519
if(supportsMutation){
2507-
// TODO: This needs to run whenever there's an insertion or update
2508-
// inside a hidden Offscreen tree.
2509-
hideOrUnhideAllChildren(finishedWork,isHidden);
2520+
// If it's trying to unhide but the parent is still hidden, then we should not unhide.
2521+
if(isHidden||!offscreenDirectParentIsHidden){
2522+
hideOrUnhideAllChildren(finishedWork,isHidden);
2523+
}
25102524
}
25112525
}
25122526

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let waitForPaint;
1414
letwaitFor;
1515
letassertLog;
1616
letassertConsoleErrorDev;
17+
letSuspense;
1718

1819
describe('Activity',()=>{
1920
beforeEach(()=>{
@@ -25,6 +26,7 @@ describe('Activity', () => {
2526
act=require('internal-test-utils').act;
2627
LegacyHidden=React.unstable_LegacyHidden;
2728
Activity=React.Activity;
29+
Suspense=React.Suspense;
2830
useState=React.useState;
2931
useInsertionEffect=React.useInsertionEffect;
3032
useLayoutEffect=React.useLayoutEffect;
@@ -1424,6 +1426,72 @@ describe('Activity', () => {
14241426
);
14251427
});
14261428

1429+
// @gate enableActivity
1430+
it('reveal an inner Activity boundary without revealing an outer one on the same host child',async()=>{
1431+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1432+
// then re-hides the child on the way up.
1433+
constmemoizedElement=<div/>;
1434+
functionApp({showOuter, showInner}){
1435+
return(
1436+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1437+
<Activitymode={showInner ? 'visible' : 'hidden'}name="Inner">
1438+
{memoizedElement}
1439+
</Activity>
1440+
</Activity>
1441+
);
1442+
}
1443+
1444+
constroot=ReactNoop.createRoot();
1445+
1446+
// Prerender the whole tree.
1447+
awaitact(()=>{
1448+
root.render(<AppshowOuter={false}showInner={false}/>);
1449+
});
1450+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1451+
1452+
awaitact(()=>{
1453+
root.render(<AppshowOuter={false}showInner={true}/>);
1454+
});
1455+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1456+
});
1457+
1458+
// @gate enableActivity
1459+
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child',async()=>{
1460+
// This ensures that no update is scheduled, which would cover up the bug if the parent
1461+
// then re-hides the child on the way up.
1462+
constmemoizedElement=<div/>;
1463+
constpromise=newPromise(()=>{});
1464+
functionApp({showOuter, showInner}){
1465+
return(
1466+
<Activitymode={showOuter ? 'visible' : 'hidden'}name="Outer">
1467+
<Suspensename="Inner">
1468+
{memoizedElement}
1469+
{showInner ? null : promise}
1470+
</Suspense>
1471+
</Activity>
1472+
);
1473+
}
1474+
1475+
constroot=ReactNoop.createRoot();
1476+
1477+
// Prerender the whole tree.
1478+
awaitact(()=>{
1479+
root.render(<AppshowOuter={false}showInner={true}/>);
1480+
});
1481+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1482+
1483+
// Resuspend the inner.
1484+
awaitact(()=>{
1485+
root.render(<AppshowOuter={false}showInner={false}/>);
1486+
});
1487+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1488+
1489+
awaitact(()=>{
1490+
root.render(<AppshowOuter={false}showInner={true}/>);
1491+
});
1492+
expect(root).toMatchRenderedOutput(<divhidden={true}/>);
1493+
});
1494+
14271495
// @gate enableActivity
14281496
it('insertion effects are not disconnected when the visibility changes',async()=>{
14291497
functionChild({step}){

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,6 +1401,105 @@ describe('ReactSuspenseEffectsSemantics', () => {
14011401
);
14021402
});
14031403

1404+
// @gate enableLegacyCache
1405+
it('should wait to reveal an inner child when inner one reveals first',async()=>{
1406+
functionApp({outerChildren, innerChildren}){
1407+
return(
1408+
<Suspensefallback={<Texttext="OuterFallback"/>}name="Outer">
1409+
<Suspensefallback={<Texttext="InnerFallback"/>}name="Inner">
1410+
<div>{innerChildren}</div>
1411+
</Suspense>
1412+
{outerChildren}
1413+
</Suspense>
1414+
);
1415+
}
1416+
1417+
// Mount
1418+
awaitact(()=>{
1419+
ReactNoop.render(<App/>);
1420+
});
1421+
assertLog([]);
1422+
expect(ReactNoop).toMatchRenderedOutput(<div/>);
1423+
1424+
// Resuspend inner boundary
1425+
awaitact(()=>{
1426+
ReactNoop.render(
1427+
<App
1428+
outerChildren={null}
1429+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1430+
/>,
1431+
);
1432+
});
1433+
assertLog([
1434+
'Suspend:InnerAsync',
1435+
'Text:InnerFallback render',
1436+
'Text:InnerFallback create insertion',
1437+
'Text:InnerFallback create layout',
1438+
'Text:InnerFallback create passive',
1439+
'Suspend:InnerAsync',
1440+
]);
1441+
expect(ReactNoop).toMatchRenderedOutput(
1442+
<>
1443+
<divhidden={true}/>
1444+
<spanprop="InnerFallback"/>
1445+
</>,
1446+
);
1447+
1448+
// Resuspend both boundaries
1449+
awaitact(()=>{
1450+
ReactNoop.render(
1451+
<App
1452+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1453+
innerChildren={<AsyncTexttext="InnerAsync"/>}
1454+
/>,
1455+
);
1456+
});
1457+
assertLog([
1458+
'Suspend:InnerAsync',
1459+
'Text:InnerFallback render',
1460+
'Suspend:OuterAsync',
1461+
'Text:OuterFallback render',
1462+
'Text:InnerFallback destroy layout',
1463+
'Text:OuterFallback create insertion',
1464+
'Text:OuterFallback create layout',
1465+
'Text:OuterFallback create passive',
1466+
'Suspend:InnerAsync',
1467+
'Text:InnerFallback render',
1468+
'Suspend:OuterAsync',
1469+
]);
1470+
expect(ReactNoop).toMatchRenderedOutput(
1471+
<>
1472+
<divhidden={true}/>
1473+
<spanprop="InnerFallback"hidden={true}/>
1474+
<spanprop="OuterFallback"/>
1475+
</>,
1476+
);
1477+
1478+
// Unsuspend the inner Suspense subtree only
1479+
// Interestingly, this never commits because the tree is left suspended.
1480+
// If it did commit, it would potentially cause the div to incorrectly reappear.
1481+
awaitact(()=>{
1482+
ReactNoop.render(
1483+
<App
1484+
outerChildren={<AsyncTexttext="OuterAsync"/>}
1485+
innerChildren={null}
1486+
/>,
1487+
);
1488+
});
1489+
assertLog([
1490+
'Suspend:OuterAsync',
1491+
'Text:OuterFallback render',
1492+
'Suspend:OuterAsync',
1493+
]);
1494+
expect(ReactNoop).toMatchRenderedOutput(
1495+
<>
1496+
<divhidden={true}/>
1497+
<spanprop="InnerFallback"hidden={true}/>
1498+
<spanprop="OuterFallback"/>
1499+
</>,
1500+
);
1501+
});
1502+
14041503
// @gate enableLegacyCache
14051504
it('should show nested host nodes if multiple boundaries resolve at the same time',async()=>{
14061505
functionApp({innerChildren =null, outerChildren =null}){

0 commit comments

Comments
 (0)