Defer more field detachments to passive phase

This allows us to use those fields during passive unmount traversal.

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;
  }

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;
  }

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<mixed>): Thenable<void> {
  };
  }
 }
-
-function detachFiberAfterEffects(fiber: Fiber): void {
- fiber.sibling = null;
- fiber.stateNode = null;
-}

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<mixed>): Thenable<void> {
  };
  }
 }
-
-function detachFiberAfterEffects(fiber: Fiber): void {
- fiber.sibling = null;
- fiber.stateNode = null;
-}