More cleanup of passive effects TODOs

* Re-added sibling/return/child fiber detaches.
* Clear deletions array properly after mutation or passive effects

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

@@ -881,17 +881,10 @@ function commitUnmount(
  if ((tag & HookPassive) !== NoHookEffect) {
  effect.tag |= HookHasEffect;
 
- // subtreeTags currently bubble in resetChildLanes which doens't get called for unmounted subtrees.
- // So we need to bubble this up manually to the root (or to the nearest ancestor that already has it).
+ // subtreeTags bubble in resetChildLanes which doens't get called for unmounted subtrees.
+ // So in the case of unmounts, we need to bubble passive effects explicitly.
  let ancestor = current.return;
  while (ancestor !== null) {
- if (
- (ancestor.subtreeTag & PassiveSubtreeTag) !==
- NoSubtreeTag
- ) {
- break;
- }
-
  ancestor.subtreeTag |= PassiveSubtreeTag;
  const alternate = ancestor.alternate;
  if (alternate !== null) {
@@ -1035,8 +1028,11 @@ 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 pointers to disconnect it from the tree. 
+ // Note that we can't clear child or sibling pointers yet,
+ // because they may be required for passive effects.
+ // These pointers will be cleared in a separate pass.
+ // Ideally, we should 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.
@@ -1045,8 +1041,6 @@ function detachFiberMutation(fiber: Fiber) {
  // traversal in a later effect. See PR #16820. We now clear the sibling
  // field after effects, see: detachFiberAfterEffects.
  fiber.alternate = null;
- // TODO (effects) Don't clear this yet because Passive effects might need it.
- //fiber.child = null;
  fiber.dependencies = null;
  fiber.firstEffect = null;
  fiber.lastEffect = null;
@@ -1055,8 +1049,6 @@ function detachFiberMutation(fiber: Fiber) {
  fiber.pendingProps = null;
  fiber.return = null;
  fiber.stateNode = null;
- // TODO (effects) Don't clear this yet because Passive effects might need it.
- //fiber.updateQueue = null;
  if (__DEV__) {
  fiber._debugOwner = null;
  }

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

@@ -2406,15 +2406,16 @@ function commitMutationEffects(
 ) {
  let fiber = firstChild;
  while (fiber !== null) {
- if (fiber.deletions !== null) {
- commitMutationEffectsDeletions(
- fiber.deletions,
- root,
- renderPriorityLevel,
- );
+ const deletions = fiber.deletions;
+ if (deletions !== null) {
+ commitMutationEffectsDeletions(deletions, root, renderPriorityLevel);
 
- // TODO (effects) Detach sibling pointers for deleted Fibers
- // TODO (effects) Clear deletion arrays
+ // If there are no pending passive effects, clear the deletions Array.
+ const primaryEffectTag = fiber.effectTag & PassiveMask;
+ const primarySubtreeTag = fiber.subtreeTag & PassiveSubtreeTag;
+ if (primaryEffectTag === NoEffect && primarySubtreeTag === NoSubtreeTag) {
+ fiber.deletions = null;
+ }
  }
 
  if (fiber.child !== null) {
@@ -2548,7 +2549,13 @@ function commitMutationEffectsDeletions(
  captureCommitPhaseError(childToDelete, error);
  }
  }
- // Don't clear the Deletion effect yet; we also use it to know when we need to detach refs later.
+
+ // If there are no pending passive effects, it's safe to detach remaining pointers now.
+ const primarySubtreeTag = childToDelete.subtreeTag & PassiveSubtreeTag;
+ const primaryEffectTag = childToDelete.effectTag & PassiveMask;
+ if (primarySubtreeTag === NoSubtreeTag && primaryEffectTag === NoEffect) {
+ detachFiberAfterEffects(childToDelete);
+ }
  }
 }
 
@@ -2777,15 +2784,21 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
  for (let i = 0; i < deletions.length; i++) {
  const fiberToDelete = deletions[i];
  // If this fiber (or anything below it) has passive effects then traverse the subtree.
- const primaryEffectTag = fiberToDelete.effectTag & (Passive | Update);
+ const primaryEffectTag = fiberToDelete.effectTag & PassiveMask;
  const primarySubtreeTag = fiberToDelete.subtreeTag & PassiveSubtreeTag;
  if (
  primarySubtreeTag !== NoSubtreeTag ||
  primaryEffectTag !== NoEffect
  ) {
  flushPassiveUnmountEffects(fiberToDelete);
  }
+
+ // Now that passive effects have been processed, it's safe to detach lingering pointers.
+ detachFiberAfterEffects(fiberToDelete);
  }
+
+ // Clear deletions now that passive effects have been procssed.
+ fiber.deletions = null;
  }
 
  const didBailout =
@@ -2809,7 +2822,7 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
  case ForwardRef:
  case SimpleMemoComponent:
  case Block: {
- const primaryEffectTag = fiber.effectTag & (Passive | Update);
+ const primaryEffectTag = fiber.effectTag & PassiveMask;
  if (primaryEffectTag !== NoEffect) {
  flushPassiveUnmountEffectsImpl(fiber);
  }
@@ -2934,8 +2947,6 @@ function flushPassiveEffectsImpl() {
  flushPassiveUnmountEffects(root.current);
  flushPassiveMountEffects(root.current);
 
- // TODO (effects) Detach sibling pointers for deleted Fibers
-
  if (enableProfilerTimer && enableProfilerCommitHooks) {
  const profilerEffects = pendingPassiveProfilerEffects;
  pendingPassiveProfilerEffects = [];
@@ -4034,3 +4045,9 @@ export function act(callback: () => Thenable<mixed>): Thenable<void> {
  };
  }
 }
+
+function detachFiberAfterEffects(fiber: Fiber): void {
+ fiber.child = null;
+ fiber.sibling = null;
+ fiber.updateQueue = null;
+}

packages/react-reconciler/src/ReactSideEffectTags.js

@@ -26,6 +26,7 @@ export const Snapshot = /* */ 0b000000100000000;
 export const Passive = /* */ 0b000001000000000;
 export const PassiveUnmountPendingDev = /* */ 0b010000000000000;
 export const Hydrating = /* */ 0b000010000000000;
+
 export const HydratingAndUpdate = /* */ 0b000010000000100;
 
 // Passive & Update & Callback & Ref & Snapshot