More cleanup of passive effects TODOs

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

Author: Brian Vaughn

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