Skip to content

Commit ca98b30

Browse files
acdliteacdlite
committed
Fix: Unmount suspended tree when it is deleted
1 parent bfc22fc commit ca98b30

File tree

2 files changed

+90
-50
lines changed

2 files changed

+90
-50
lines changed

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

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,24 +1804,36 @@ function commitDeletionEffectsOnFiber(
18041804
return;
18051805
}
18061806
case OffscreenComponent: {
1807-
// If this offscreen component is hidden, we already unmounted it. Before
1808-
// deleting the children, track that it's already unmounted so that we
1809-
// don't attempt to unmount the effects again.
1810-
// TODO: If the tree is hidden, in most cases we should be able to skip
1811-
// over the nested children entirely. An exception is we haven't yet found
1812-
// the topmost host node to delete, which we already track on the stack.
1813-
// But the other case is portals, which need to be detached no matter how
1814-
// deeply they are nested. We should use a subtree flag to track whether a
1815-
// subtree includes a nested portal.
1816-
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
1817-
offscreenSubtreeWasHidden =
1818-
prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null;
1819-
recursivelyTraverseDeletionEffects(
1820-
finishedRoot,
1821-
nearestMountedAncestor,
1822-
deletedFiber,
1823-
);
1824-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
1807+
if (
1808+
// TODO: Remove this dead flag
1809+
enableSuspenseLayoutEffectSemantics &&
1810+
deletedFiber.mode & ConcurrentMode
1811+
) {
1812+
// If this offscreen component is hidden, we already unmounted it. Before
1813+
// deleting the children, track that it's already unmounted so that we
1814+
// don't attempt to unmount the effects again.
1815+
// TODO: If the tree is hidden, in most cases we should be able to skip
1816+
// over the nested children entirely. An exception is we haven't yet found
1817+
// the topmost host node to delete, which we already track on the stack.
1818+
// But the other case is portals, which need to be detached no matter how
1819+
// deeply they are nested. We should use a subtree flag to track whether a
1820+
// subtree includes a nested portal.
1821+
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
1822+
offscreenSubtreeWasHidden =
1823+
prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null;
1824+
recursivelyTraverseDeletionEffects(
1825+
finishedRoot,
1826+
nearestMountedAncestor,
1827+
deletedFiber,
1828+
);
1829+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
1830+
} else {
1831+
recursivelyTraverseDeletionEffects(
1832+
finishedRoot,
1833+
nearestMountedAncestor,
1834+
deletedFiber,
1835+
);
1836+
}
18251837
break;
18261838
}
18271839
default: {
@@ -2238,13 +2250,21 @@ function commitMutationEffectsOnFiber(
22382250
case OffscreenComponent: {
22392251
const wasHidden = current !== null && current.memoizedState !== null;
22402252

2241-
// Before committing the children, track on the stack whether this
2242-
// offscreen subtree was already hidden, so that we don't unmount the
2243-
// effects again.
2244-
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2245-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
2246-
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
2247-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2253+
if (
2254+
// TODO: Remove this dead flag
2255+
enableSuspenseLayoutEffectSemantics &&
2256+
finishedWork.mode & ConcurrentMode
2257+
) {
2258+
// Before committing the children, track on the stack whether this
2259+
// offscreen subtree was already hidden, so that we don't unmount the
2260+
// effects again.
2261+
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2262+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
2263+
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
2264+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2265+
} else {
2266+
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
2267+
}
22482268

22492269
commitReconciliationEffects(finishedWork);
22502270

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

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,24 +1804,36 @@ function commitDeletionEffectsOnFiber(
18041804
return;
18051805
}
18061806
case OffscreenComponent: {
1807-
// If this offscreen component is hidden, we already unmounted it. Before
1808-
// deleting the children, track that it's already unmounted so that we
1809-
// don't attempt to unmount the effects again.
1810-
// TODO: If the tree is hidden, in most cases we should be able to skip
1811-
// over the nested children entirely. An exception is we haven't yet found
1812-
// the topmost host node to delete, which we already track on the stack.
1813-
// But the other case is portals, which need to be detached no matter how
1814-
// deeply they are nested. We should use a subtree flag to track whether a
1815-
// subtree includes a nested portal.
1816-
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
1817-
offscreenSubtreeWasHidden =
1818-
prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null;
1819-
recursivelyTraverseDeletionEffects(
1820-
finishedRoot,
1821-
nearestMountedAncestor,
1822-
deletedFiber,
1823-
);
1824-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
1807+
if (
1808+
// TODO: Remove this dead flag
1809+
enableSuspenseLayoutEffectSemantics &&
1810+
deletedFiber.mode & ConcurrentMode
1811+
) {
1812+
// If this offscreen component is hidden, we already unmounted it. Before
1813+
// deleting the children, track that it's already unmounted so that we
1814+
// don't attempt to unmount the effects again.
1815+
// TODO: If the tree is hidden, in most cases we should be able to skip
1816+
// over the nested children entirely. An exception is we haven't yet found
1817+
// the topmost host node to delete, which we already track on the stack.
1818+
// But the other case is portals, which need to be detached no matter how
1819+
// deeply they are nested. We should use a subtree flag to track whether a
1820+
// subtree includes a nested portal.
1821+
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
1822+
offscreenSubtreeWasHidden =
1823+
prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null;
1824+
recursivelyTraverseDeletionEffects(
1825+
finishedRoot,
1826+
nearestMountedAncestor,
1827+
deletedFiber,
1828+
);
1829+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
1830+
} else {
1831+
recursivelyTraverseDeletionEffects(
1832+
finishedRoot,
1833+
nearestMountedAncestor,
1834+
deletedFiber,
1835+
);
1836+
}
18251837
break;
18261838
}
18271839
default: {
@@ -2238,13 +2250,21 @@ function commitMutationEffectsOnFiber(
22382250
case OffscreenComponent: {
22392251
const wasHidden = current !== null && current.memoizedState !== null;
22402252

2241-
// Before committing the children, track on the stack whether this
2242-
// offscreen subtree was already hidden, so that we don't unmount the
2243-
// effects again.
2244-
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2245-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
2246-
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
2247-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2253+
if (
2254+
// TODO: Remove this dead flag
2255+
enableSuspenseLayoutEffectSemantics &&
2256+
finishedWork.mode & ConcurrentMode
2257+
) {
2258+
// Before committing the children, track on the stack whether this
2259+
// offscreen subtree was already hidden, so that we don't unmount the
2260+
// effects again.
2261+
const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
2262+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
2263+
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
2264+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2265+
} else {
2266+
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
2267+
}
22482268

22492269
commitReconciliationEffects(finishedWork);
22502270

0 commit comments

Comments
 (0)