Skip to content

Commit 4bd245e

Browse files
authored
Do not unmount layout effects if ancestor Offscreen is hidden (#25628)
This is a follow up on #25592 There is another condition Offscreen calls `recursivelyTraverseDisappearLayoutEffects` when it shouldn't. Offscreen may be nested. When nested Offscreen is hidden, it should only unmount layout effects if it meets following conditions: 1. This is an update, not first mount. 2. This Offscreen was hidden before. 3. No ancestor Offscreen is hidden. Previously, we were not accounting for the third condition.
1 parent df61e70 commit 4bd245e

3 files changed

Lines changed: 46 additions & 14 deletions

File tree

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2869,7 +2869,6 @@ function commitMutationEffectsOnFiber(
28692869

28702870
if(flags&Visibility){
28712871
constoffscreenInstance: OffscreenInstance=finishedWork.stateNode;
2872-
constoffscreenBoundary: Fiber=finishedWork;
28732872

28742873
// Track the current state on the Offscreen instance so we can
28752874
// read it during an event
@@ -2880,11 +2879,16 @@ function commitMutationEffectsOnFiber(
28802879
}
28812880

28822881
if(isHidden){
2883-
// Check if this is an update, and the tree was previously visible.
2884-
if(current!==null&&!wasHidden){
2885-
if((offscreenBoundary.mode&ConcurrentMode)!==NoMode){
2882+
constisUpdate=current!==null;
2883+
constisAncestorOffscreenHidden=offscreenSubtreeIsHidden;
2884+
// Only trigger disapper layout effects if:
2885+
// - This is an update, not first mount.
2886+
// - This Offscreen was not hidden before.
2887+
// - No ancestor Offscreen is hidden.
2888+
if(isUpdate&&!wasHidden&&!isAncestorOffscreenHidden){
2889+
if((finishedWork.mode&ConcurrentMode)!==NoMode){
28862890
// Disappear the layout effects of all the children
2887-
recursivelyTraverseDisappearLayoutEffects(offscreenBoundary);
2891+
recursivelyTraverseDisappearLayoutEffects(finishedWork);
28882892
}
28892893
}
28902894
}else{
@@ -2897,7 +2901,7 @@ function commitMutationEffectsOnFiber(
28972901
if(supportsMutation&&!isOffscreenManual(finishedWork)){
28982902
// TODO: This needs to run whenever there's an insertion or update
28992903
// inside a hidden Offscreen tree.
2900-
hideOrUnhideAllChildren(offscreenBoundary,isHidden);
2904+
hideOrUnhideAllChildren(finishedWork,isHidden);
29012905
}
29022906
}
29032907

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2869,7 +2869,6 @@ function commitMutationEffectsOnFiber(
28692869

28702870
if(flags&Visibility){
28712871
constoffscreenInstance: OffscreenInstance=finishedWork.stateNode;
2872-
constoffscreenBoundary: Fiber=finishedWork;
28732872

28742873
// Track the current state on the Offscreen instance so we can
28752874
// read it during an event
@@ -2880,11 +2879,16 @@ function commitMutationEffectsOnFiber(
28802879
}
28812880

28822881
if(isHidden){
2883-
// Check if this is an update, and the tree was previously visible.
2884-
if(current!==null&&!wasHidden){
2885-
if((offscreenBoundary.mode&ConcurrentMode)!==NoMode){
2882+
constisUpdate=current!==null;
2883+
constisAncestorOffscreenHidden=offscreenSubtreeIsHidden;
2884+
// Only trigger disapper layout effects if:
2885+
// - This is an update, not first mount.
2886+
// - This Offscreen was not hidden before.
2887+
// - No ancestor Offscreen is hidden.
2888+
if(isUpdate&&!wasHidden&&!isAncestorOffscreenHidden){
2889+
if((finishedWork.mode&ConcurrentMode)!==NoMode){
28862890
// Disappear the layout effects of all the children
2887-
recursivelyTraverseDisappearLayoutEffects(offscreenBoundary);
2891+
recursivelyTraverseDisappearLayoutEffects(finishedWork);
28882892
}
28892893
}
28902894
}else{
@@ -2897,7 +2901,7 @@ function commitMutationEffectsOnFiber(
28972901
if(supportsMutation&&!isOffscreenManual(finishedWork)){
28982902
// TODO: This needs to run whenever there's an insertion or update
28992903
// inside a hidden Offscreen tree.
2900-
hideOrUnhideAllChildren(offscreenBoundary,isHidden);
2904+
hideOrUnhideAllChildren(finishedWork,isHidden);
29012905
}
29022906
}
29032907

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,21 @@ describe('ReactOffscreen', () => {
281281
componentWillUnmount(){
282282
Scheduler.unstable_yieldValue('componentWillUnmount');
283283
}
284+
285+
componentDidMount(){
286+
Scheduler.unstable_yieldValue('componentDidMount');
287+
}
284288
}
285289

290+
let_setIsVisible;
291+
letisFirstRender=true;
292+
286293
functionApp(){
287294
const[isVisible,setIsVisible]=React.useState(true);
288-
289-
if(isVisible===true){
295+
_setIsVisible=setIsVisible;
296+
if(isFirstRender===true){
290297
setIsVisible(false);
298+
isFirstRender=false;
291299
}
292300

293301
return(
@@ -303,7 +311,23 @@ describe('ReactOffscreen', () => {
303311
awaitact(async()=>{
304312
root.render(<App/>);
305313
});
314+
315+
expect(Scheduler).toHaveYielded(['Child']);
316+
expect(root).toMatchRenderedOutput(<spanhidden={true}prop="Child"/>);
317+
318+
awaitact(async()=>{
319+
_setIsVisible(true);
320+
});
321+
322+
expect(Scheduler).toHaveYielded(['Child']);
323+
expect(root).toMatchRenderedOutput(<spanhidden={true}prop="Child"/>);
324+
325+
awaitact(async()=>{
326+
_setIsVisible(false);
327+
});
328+
306329
expect(Scheduler).toHaveYielded(['Child']);
330+
expect(root).toMatchRenderedOutput(<spanhidden={true}prop="Child"/>);
307331
});
308332

309333
// @gate enableOffscreen

0 commit comments

Comments
 (0)