Skip to content

Commit

Permalink
Make sure Offscreen's ref is detached when unmounted
Browse files Browse the repository at this point in the history
  • Loading branch information
sammy-SC committed Sep 21, 2022
1 parent a8f7a96 commit 1608017
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 2 deletions.
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ function commitLayoutEffectOnFiber(
if (flags & Ref) {
safelyAttachRef(finishedWork, finishedWork.return);
}
} else if (finishedWork.pendingProps.mode !== undefined) {
} else {
safelyDetachRef(finishedWork, finishedWork.return);
}
} else {
Expand Down Expand Up @@ -2148,6 +2148,8 @@ function commitDeletionEffectsOnFiber(
offscreenSubtreeWasHidden =
prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null;

safelyDetachRef(deletedFiber, nearestMountedAncestor);

recursivelyTraverseDeletionEffects(
finishedRoot,
nearestMountedAncestor,
Expand Down Expand Up @@ -2833,6 +2835,7 @@ export function disappearLayoutEffects(finishedWork: Fiber) {
// Nested Offscreen tree is already hidden. Don't disappear
// its effects.
} else {
safelyDetachRef(finishedWork, finishedWork.return);
recursivelyTraverseDisappearLayoutEffects(finishedWork);
}
break;
Expand Down Expand Up @@ -2974,6 +2977,8 @@ export function reappearLayoutEffects(
finishedWork,
includeWorkInProgressEffects,
);

safelyAttachRef(finishedWork, finishedWork.return);
}
break;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ function commitLayoutEffectOnFiber(
if (flags & Ref) {
safelyAttachRef(finishedWork, finishedWork.return);
}
} else if (finishedWork.pendingProps.mode !== undefined) {
} else {
safelyDetachRef(finishedWork, finishedWork.return);
}
} else {
Expand Down Expand Up @@ -2148,6 +2148,8 @@ function commitDeletionEffectsOnFiber(
offscreenSubtreeWasHidden =
prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null;

safelyDetachRef(deletedFiber, nearestMountedAncestor);

recursivelyTraverseDeletionEffects(
finishedRoot,
nearestMountedAncestor,
Expand Down Expand Up @@ -2833,6 +2835,7 @@ export function disappearLayoutEffects(finishedWork: Fiber) {
// Nested Offscreen tree is already hidden. Don't disappear
// its effects.
} else {
safelyDetachRef(finishedWork, finishedWork.return);
recursivelyTraverseDisappearLayoutEffects(finishedWork);
}
break;
Expand Down Expand Up @@ -2974,6 +2977,8 @@ export function reappearLayoutEffects(
finishedWork,
includeWorkInProgressEffects,
);

safelyAttachRef(finishedWork, finishedWork.return);
}
break;
}
Expand Down
80 changes: 80 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1307,4 +1307,84 @@ describe('ReactOffscreen', () => {
expect(offscreenRef.current).not.toBeNull();
});
});

// @gate enableOffscreen
it('should detach ref if Offscreen is unmounted', async () => {
let offscreenRef;

function App({showOffscreen}) {
offscreenRef = useRef(null);
return showOffscreen ? (
<Offscreen
mode={'manual'}
ref={ref => {
offscreenRef.current = ref;
}}>
<div />
</Offscreen>
) : null;
}

const root = ReactNoop.createRoot();

await act(async () => {
root.render(<App showOffscreen={true} />);
});

expect(offscreenRef.current).not.toBeNull();

await act(async () => {
root.render(<App showOffscreen={false} />);
});

expect(offscreenRef.current).toBeNull();

await act(async () => {
root.render(<App showOffscreen={true} />);
});

expect(offscreenRef.current).not.toBeNull();
});

// @gate enableOffscreen
it('should detach ref if Offscreen is unmounted', async () => {
let offscreenRef;
let divRef;

function App({mode}) {
offscreenRef = useRef(null);
divRef = useRef(null);
return (
<Offscreen mode={mode}>
<Offscreen mode={'manual'} ref={offscreenRef}>
<div ref={divRef} />
</Offscreen>
</Offscreen>
);
}

const root = ReactNoop.createRoot();

await act(async () => {
root.render(<App mode={'hidden'} />);
});

expect(offscreenRef.current).toBeNull();
expect(divRef.current).toBeNull();

await act(async () => {
root.render(<App mode={'visible'} />);
});

expect(offscreenRef.current).not.toBeNull();
expect(divRef.current).not.toBeNull();

console.log('Becoming Hidden');
await act(async () => {
root.render(<App mode={'hidden'} />);
});

expect(offscreenRef.current).toBeNull();
expect(divRef.current).toBeNull();
});
});

0 comments on commit 1608017

Please sign in to comment.