diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index fb4c942ab6eea..a3147ab213c45 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1069,30 +1069,42 @@ function commitNestedUnmounts( } function detachFiberMutation(fiber: Fiber) { - // Cut off the return pointers to disconnect it from the tree. Ideally, we - // should clear the child pointer of the parent alternate to let this + // Cut off the return pointer to disconnect it from the tree. + // This enables us to detect and warn against state updates on an unmounted component. + // It also prevents events from bubbling from within disconnected components. + // + // Ideally, we should also clear the child pointer of the parent alternate to let this // get GC:ed but we don't know which for sure which parent is the current - // one so we'll settle for GC:ing the subtree of this child. This child - // itself will be GC:ed when the parent updates the next time. - // Note: we cannot null out sibling here, otherwise it can cause issues - // with findDOMNode and how it requires the sibling field to carry out - // traversal in a later effect. See PR #16820. We now clear the sibling - // field after effects, see: detachFiberAfterEffects. + // one so we'll settle for GC:ing the subtree of this child. + // This child itself will be GC:ed when the parent updates the next time. // - // Don't disconnect stateNode now; it will be detached in detachFiberAfterEffects. - // It may be required if the current component is an error boundary, - // and one of its descendants throws while unmounting a passive effect. - fiber.alternate = null; + // Note that we can't clear child or sibling pointers yet. + // They're needed for passive effects and for findDOMNode. + // We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects). + const alternate = fiber.alternate; + if (alternate !== null) { + alternate.return = null; + fiber.alternate = null; + } + fiber.return = null; +} + +export function detachFiberAfterEffects(fiber: Fiber): void { + // Null out fields to improve GC for references that may be lingering (e.g. DevTools). + // Note that we already cleared the return pointer in detachFiberMutation(). fiber.child = null; fiber.deletions = null; fiber.dependencies = null; - fiber.firstEffect = null; - fiber.lastEffect = null; fiber.memoizedProps = null; fiber.memoizedState = null; fiber.pendingProps = null; - fiber.return = null; + fiber.sibling = null; + fiber.stateNode = null; fiber.updateQueue = null; + fiber.nextEffect = null; + fiber.firstEffect = null; + fiber.lastEffect = null; + if (__DEV__) { fiber._debugOwner = null; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 7f98172a30edb..6d7776597c158 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1069,30 +1069,42 @@ function commitNestedUnmounts( } function detachFiberMutation(fiber: Fiber) { - // Cut off the return pointers to disconnect it from the tree. Ideally, we - // should clear the child pointer of the parent alternate to let this + // Cut off the return pointer to disconnect it from the tree. + // This enables us to detect and warn against state updates on an unmounted component. + // It also prevents events from bubbling from within disconnected components. + // + // Ideally, we should also clear the child pointer of the parent alternate to let this // get GC:ed but we don't know which for sure which parent is the current - // one so we'll settle for GC:ing the subtree of this child. This child - // itself will be GC:ed when the parent updates the next time. - // Note: we cannot null out sibling here, otherwise it can cause issues - // with findDOMNode and how it requires the sibling field to carry out - // traversal in a later effect. See PR #16820. We now clear the sibling - // field after effects, see: detachFiberAfterEffects. + // one so we'll settle for GC:ing the subtree of this child. + // This child itself will be GC:ed when the parent updates the next time. // - // Don't disconnect stateNode now; it will be detached in detachFiberAfterEffects. - // It may be required if the current component is an error boundary, - // and one of its descendants throws while unmounting a passive effect. - fiber.alternate = null; + // Note that we can't clear child or sibling pointers yet. + // They're needed for passive effects and for findDOMNode. + // We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects). + const alternate = fiber.alternate; + if (alternate !== null) { + alternate.return = null; + fiber.alternate = null; + } + fiber.return = null; +} + +export function detachFiberAfterEffects(fiber: Fiber): void { + // Null out fields to improve GC for references that may be lingering (e.g. DevTools). + // Note that we already cleared the return pointer in detachFiberMutation(). fiber.child = null; fiber.deletions = null; fiber.dependencies = null; - fiber.firstEffect = null; - fiber.lastEffect = null; fiber.memoizedProps = null; fiber.memoizedState = null; fiber.pendingProps = null; - fiber.return = null; + fiber.sibling = null; + fiber.stateNode = null; fiber.updateQueue = null; + fiber.nextEffect = null; + fiber.firstEffect = null; + fiber.lastEffect = null; + if (__DEV__) { fiber._debugOwner = null; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 3177abb63c511..a994a01c204ed 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -122,6 +122,7 @@ import { Update, PlacementAndUpdate, Deletion, + ChildDeletion, Ref, ContentReset, Snapshot, @@ -194,6 +195,7 @@ import { commitResetTextContent, isSuspenseBoundaryBeingHidden, commitPassiveMountEffects, + detachFiberAfterEffects, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactUpdateQueue.new'; import {resetContextDependencies} from './ReactFiberNewContext.new'; @@ -2129,13 +2131,21 @@ function commitRootImpl(root, renderPriorityLevel) { } else { // We are done with the effect chain at this point so let's clear the // nextEffect pointers to assist with GC. If we have passive effects, we'll - // clear this in flushPassiveEffects. + // clear this in flushPassiveEffects + // TODO: We should always do this in the passive phase, by scheduling + // a passive callback for every deletion. nextEffect = firstEffect; while (nextEffect !== null) { const nextNextEffect = nextEffect.nextEffect; nextEffect.nextEffect = null; - if (nextEffect.flags & Deletion) { - detachFiberAfterEffects(nextEffect); + if (nextEffect.flags & ChildDeletion) { + const deletions = nextEffect.deletions; + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const deletion = deletions[i]; + detachFiberAfterEffects(deletion); + } + } } nextEffect = nextNextEffect; } @@ -3708,8 +3718,3 @@ export function act(callback: () => Thenable): Thenable { }; } } - -function detachFiberAfterEffects(fiber: Fiber): void { - fiber.sibling = null; - fiber.stateNode = null; -} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index dc21ff6b7f3ed..020d90cf56104 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -122,6 +122,7 @@ import { Update, PlacementAndUpdate, Deletion, + ChildDeletion, Ref, ContentReset, Snapshot, @@ -194,6 +195,7 @@ import { commitResetTextContent, isSuspenseBoundaryBeingHidden, commitPassiveMountEffects, + detachFiberAfterEffects, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactUpdateQueue.old'; import {resetContextDependencies} from './ReactFiberNewContext.old'; @@ -2129,13 +2131,21 @@ function commitRootImpl(root, renderPriorityLevel) { } else { // We are done with the effect chain at this point so let's clear the // nextEffect pointers to assist with GC. If we have passive effects, we'll - // clear this in flushPassiveEffects. + // clear this in flushPassiveEffects + // TODO: We should always do this in the passive phase, by scheduling + // a passive callback for every deletion. nextEffect = firstEffect; while (nextEffect !== null) { const nextNextEffect = nextEffect.nextEffect; nextEffect.nextEffect = null; - if (nextEffect.flags & Deletion) { - detachFiberAfterEffects(nextEffect); + if (nextEffect.flags & ChildDeletion) { + const deletions = nextEffect.deletions; + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const deletion = deletions[i]; + detachFiberAfterEffects(deletion); + } + } } nextEffect = nextNextEffect; } @@ -3708,8 +3718,3 @@ export function act(callback: () => Thenable): Thenable { }; } } - -function detachFiberAfterEffects(fiber: Fiber): void { - fiber.sibling = null; - fiber.stateNode = null; -}