Convert mutation phase to depth-first traversal (#20596)

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

@@ -282,22 +282,13 @@ function ChildReconciler(shouldTrackSideEffects) {
     childToDelete.nextEffect = null;
     childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion;
 
-    let deletions = returnFiber.deletions;
+    const deletions = returnFiber.deletions;
     if (deletions === null) {
-      deletions = returnFiber.deletions = [childToDelete];
+      returnFiber.deletions = [childToDelete];
       returnFiber.flags |= ChildDeletion;
     } else {
       deletions.push(childToDelete);
     }
-    // Stash a reference to the return fiber's deletion array on each of the
-    // deleted children. This is really weird, but it's a temporary workaround
-    // while we're still using the effect list to traverse effect fibers. A
-    // better workaround would be to follow the `.return` pointer in the commit
-    // phase, but unfortunately we can't assume that `.return` points to the
-    // correct fiber, even in the commit phase, because `findDOMNode` might
-    // mutate it.
-    // TODO: Remove this line.
-    childToDelete.deletions = deletions;
   }
 
   function deleteRemainingChildren(

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

@@ -2203,14 +2203,13 @@ function updateSuspensePrimaryChildren(
     currentFallbackChildFragment.flags =
       (currentFallbackChildFragment.flags & StaticMask) | Deletion;
     workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
-    let deletions = workInProgress.deletions;
+    const deletions = workInProgress.deletions;
     if (deletions === null) {
-      deletions = workInProgress.deletions = [currentFallbackChildFragment];
+      workInProgress.deletions = [currentFallbackChildFragment];
       workInProgress.flags |= ChildDeletion;
     } else {
       deletions.push(currentFallbackChildFragment);
     }
-    currentFallbackChildFragment.deletions = deletions;
   }
 
   workInProgress.child = primaryChildFragment;
@@ -3194,14 +3193,13 @@ function remountFiber(
     current.nextEffect = null;
     current.flags = (current.flags & StaticMask) | Deletion;
 
-    let deletions = returnFiber.deletions;
+    const deletions = returnFiber.deletions;
     if (deletions === null) {
-      deletions = returnFiber.deletions = [current];
+      returnFiber.deletions = [current];
       returnFiber.flags |= ChildDeletion;
     } else {
       deletions.push(current);
     }
-    current.deletions = deletions;
 
     newWorkInProgress.flags |= Placement;
 

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

@@ -66,12 +66,16 @@ import {
   NoFlags,
   ContentReset,
   Placement,
+  PlacementAndUpdate,
   ChildDeletion,
   Snapshot,
   Update,
   Callback,
   Ref,
+  Hydrating,
+  HydratingAndUpdate,
   Passive,
+  MutationMask,
   PassiveMask,
   LayoutMask,
   PassiveUnmountPendingDev,
@@ -1841,6 +1845,172 @@ function commitResetTextContent(current: Fiber) {
   resetTextContent(current.stateNode);
 }
 
+export function commitMutationEffects(
+  root: FiberRoot,
+  renderPriorityLevel: ReactPriorityLevel,
+  firstChild: Fiber,
+) {
+  nextEffect = firstChild;
+  commitMutationEffects_begin(root, renderPriorityLevel);
+}
+
+function commitMutationEffects_begin(
+  root: FiberRoot,
+  renderPriorityLevel: ReactPriorityLevel,
+) {
+  while (nextEffect !== null) {
+    const fiber = nextEffect;
+
+    // TODO: Should wrap this in flags check, too, as optimization
+    const deletions = fiber.deletions;
+    if (deletions !== null) {
+      for (let i = 0; i < deletions.length; i++) {
+        const childToDelete = deletions[i];
+        if (__DEV__) {
+          invokeGuardedCallback(
+            null,
+            commitDeletion,
+            null,
+            root,
+            childToDelete,
+            renderPriorityLevel,
+          );
+          if (hasCaughtError()) {
+            const error = clearCaughtError();
+            captureCommitPhaseError(childToDelete, error);
+          }
+        } else {
+          try {
+            commitDeletion(root, childToDelete, renderPriorityLevel);
+          } catch (error) {
+            captureCommitPhaseError(childToDelete, error);
+          }
+        }
+      }
+    }
+
+    const child = fiber.child;
+    if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
+      ensureCorrectReturnPointer(child, fiber);
+      nextEffect = child;
+    } else {
+      commitMutationEffects_complete(root, renderPriorityLevel);
+    }
+  }
+}
+
+function commitMutationEffects_complete(
+  root: FiberRoot,
+  renderPriorityLevel: ReactPriorityLevel,
+) {
+  while (nextEffect !== null) {
+    const fiber = nextEffect;
+    if (__DEV__) {
+      setCurrentDebugFiberInDEV(fiber);
+      invokeGuardedCallback(
+        null,
+        commitMutationEffectsOnFiber,
+        null,
+        fiber,
+        root,
+        renderPriorityLevel,
+      );
+      if (hasCaughtError()) {
+        const error = clearCaughtError();
+        captureCommitPhaseError(fiber, error);
+      }
+      resetCurrentDebugFiberInDEV();
+    } else {
+      try {
+        commitMutationEffectsOnFiber(fiber, root, renderPriorityLevel);
+      } catch (error) {
+        captureCommitPhaseError(fiber, error);
+      }
+    }
+
+    const sibling = fiber.sibling;
+    if (sibling !== null) {
+      ensureCorrectReturnPointer(sibling, fiber.return);
+      nextEffect = sibling;
+      return;
+    }
+
+    nextEffect = fiber.return;
+  }
+}
+
+function commitMutationEffectsOnFiber(
+  finishedWork: Fiber,
+  root: FiberRoot,
+  renderPriorityLevel: ReactPriorityLevel,
+) {
+  const flags = finishedWork.flags;
+
+  if (flags & ContentReset) {
+    commitResetTextContent(finishedWork);
+  }
+
+  if (flags & Ref) {
+    const current = finishedWork.alternate;
+    if (current !== null) {
+      commitDetachRef(current);
+    }
+    if (enableScopeAPI) {
+      // TODO: This is a temporary solution that allowed us to transition away
+      // from React Flare on www.
+      if (finishedWork.tag === ScopeComponent) {
+        commitAttachRef(finishedWork);
+      }
+    }
+  }
+
+  // The following switch statement is only concerned about placement,
+  // updates, and deletions. To avoid needing to add a case for every possible
+  // bitmap value, we remove the secondary effects from the effect tag and
+  // switch on that value.
+  const primaryFlags = flags & (Placement | Update | Hydrating);
+  outer: switch (primaryFlags) {
+    case Placement: {
+      commitPlacement(finishedWork);
+      // Clear the "placement" from effect tag so that we know that this is
+      // inserted, before any life-cycles like componentDidMount gets called.
+      // TODO: findDOMNode doesn't rely on this any more but isMounted does
+      // and isMounted is deprecated anyway so we should be able to kill this.
+      finishedWork.flags &= ~Placement;
+      break;
+    }
+    case PlacementAndUpdate: {
+      // Placement
+      commitPlacement(finishedWork);
+      // Clear the "placement" from effect tag so that we know that this is
+      // inserted, before any life-cycles like componentDidMount gets called.
+      finishedWork.flags &= ~Placement;
+
+      // Update
+      const current = finishedWork.alternate;
+      commitWork(current, finishedWork);
+      break;
+    }
+    case Hydrating: {
+      finishedWork.flags &= ~Hydrating;
+      break;
+    }
+    case HydratingAndUpdate: {
+      finishedWork.flags &= ~Hydrating;
+
+      // Update
+      const current = finishedWork.alternate;
+      commitWork(current, finishedWork);
+      break;
+    }
+    case Update: {
+      const current = finishedWork.alternate;
+      commitWork(current, finishedWork);
+      break;
+    }
+  }
+}
+
 export function commitLayoutEffects(
   finishedWork: Fiber,
   root: FiberRoot,

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

@@ -144,14 +144,13 @@ function deleteHydratableInstance(
     returnFiber.firstEffect = returnFiber.lastEffect = childToDelete;
   }
 
-  let deletions = returnFiber.deletions;
+  const deletions = returnFiber.deletions;
   if (deletions === null) {
-    deletions = returnFiber.deletions = [childToDelete];
+    returnFiber.deletions = [childToDelete];
     returnFiber.flags |= ChildDeletion;
   } else {
     deletions.push(childToDelete);
   }
-  childToDelete.deletions = deletions;
 }
 
 function insertNonHydratedInstance(returnFiber: Fiber, fiber: Fiber) {

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

@@ -31,7 +31,6 @@ import {
   decoupleUpdatePriorityFromScheduler,
   enableDebugTracing,
   enableSchedulingProfiler,
-  enableScopeAPI,
   disableSchedulerTimeoutInWorkLoop,
 } from 'shared/ReactFeatureFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
@@ -116,7 +115,6 @@ import {
   ForwardRef,
   MemoComponent,
   SimpleMemoComponent,
-  ScopeComponent,
   Profiler,
 } from './ReactWorkTags';
 import {LegacyRoot} from './ReactRootTags';
@@ -124,19 +122,14 @@ import {
   NoFlags,
   PerformedWork,
   Placement,
-  Update,
-  PlacementAndUpdate,
   Deletion,
   ChildDeletion,
-  Ref,
-  ContentReset,
   Snapshot,
   Passive,
   PassiveStatic,
   Incomplete,
   HostEffectMask,
   Hydrating,
-  HydratingAndUpdate,
   StaticMask,
 } from './ReactFiberFlags';
 import {
@@ -190,13 +183,8 @@ import {
 import {
   commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber,
   commitLayoutEffects,
-  commitPlacement,
-  commitWork,
-  commitDeletion,
-  commitDetachRef,
-  commitAttachRef,
+  commitMutationEffects,
   commitPassiveEffectDurations,
-  commitResetTextContent,
   isSuspenseBoundaryBeingHidden,
   commitPassiveMountEffects,
   commitPassiveUnmountEffects,
@@ -2031,32 +2019,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     }
 
     // The next phase is the mutation phase, where we mutate the host tree.
-    nextEffect = firstEffect;
-    do {
-      if (__DEV__) {
-        invokeGuardedCallback(
-          null,
-          commitMutationEffects,
-          null,
-          root,
-          renderPriorityLevel,
-        );
-        if (hasCaughtError()) {
-          invariant(nextEffect !== null, 'Should be working on an effect.');
-          const error = clearCaughtError();
-          captureCommitPhaseError(nextEffect, error);
-          nextEffect = nextEffect.nextEffect;
-        }
-      } else {
-        try {
-          commitMutationEffects(root, renderPriorityLevel);
-        } catch (error) {
-          invariant(nextEffect !== null, 'Should be working on an effect.');
-          captureCommitPhaseError(nextEffect, error);
-          nextEffect = nextEffect.nextEffect;
-        }
-      }
-    } while (nextEffect !== null);
+    commitMutationEffects(root, renderPriorityLevel, finishedWork);
 
     if (shouldFireAfterActiveInstanceBlur) {
       afterActiveInstanceBlur();
@@ -2304,117 +2267,6 @@ function commitBeforeMutationEffects() {
   }
 }
 
-function commitMutationEffects(
-  root: FiberRoot,
-  renderPriorityLevel: ReactPriorityLevel,
-) {
-  // TODO: Should probably move the bulk of this function to commitWork.
-  while (nextEffect !== null) {
-    setCurrentDebugFiberInDEV(nextEffect);
-
-    const flags = nextEffect.flags;
-
-    if (flags & ContentReset) {
-      commitResetTextContent(nextEffect);
-    }
-
-    if (flags & Ref) {
-      const current = nextEffect.alternate;
-      if (current !== null) {
-        commitDetachRef(current);
-      }
-      if (enableScopeAPI) {
-        // TODO: This is a temporary solution that allowed us to transition away
-        // from React Flare on www.
-        if (nextEffect.tag === ScopeComponent) {
-          commitAttachRef(nextEffect);
-        }
-      }
-    }
-
-    // The following switch statement is only concerned about placement,
-    // updates, and deletions. To avoid needing to add a case for every possible
-    // bitmap value, we remove the secondary effects from the effect tag and
-    // switch on that value.
-    const primaryFlags = flags & (Placement | Update | Deletion | Hydrating);
-    outer: switch (primaryFlags) {
-      case Placement: {
-        commitPlacement(nextEffect);
-        // Clear the "placement" from effect tag so that we know that this is
-        // inserted, before any life-cycles like componentDidMount gets called.
-        // TODO: findDOMNode doesn't rely on this any more but isMounted does
-        // and isMounted is deprecated anyway so we should be able to kill this.
-        nextEffect.flags &= ~Placement;
-        break;
-      }
-      case PlacementAndUpdate: {
-        // Placement
-        commitPlacement(nextEffect);
-        // Clear the "placement" from effect tag so that we know that this is
-        // inserted, before any life-cycles like componentDidMount gets called.
-        nextEffect.flags &= ~Placement;
-
-        // Update
-        const current = nextEffect.alternate;
-        commitWork(current, nextEffect);
-        break;
-      }
-      case Hydrating: {
-        nextEffect.flags &= ~Hydrating;
-        break;
-      }
-      case HydratingAndUpdate: {
-        nextEffect.flags &= ~Hydrating;
-
-        // Update
-        const current = nextEffect.alternate;
-        commitWork(current, nextEffect);
-        break;
-      }
-      case Update: {
-        const current = nextEffect.alternate;
-        commitWork(current, nextEffect);
-        break;
-      }
-      case Deletion: {
-        // Reached a deletion effect. Instead of commit this effect like we
-        // normally do, we're going to use the `deletions` array of the parent.
-        // However, because the effect list is sorted in depth-first order, we
-        // can't wait until we reach the parent node, because the child effects
-        // will have run in the meantime.
-        //
-        // So instead, we use a trick where the first time we hit a deletion
-        // effect, we commit all the deletion effects that belong to that parent.
-        //
-        // This is an incremental step away from using the effect list and
-        // toward a DFS + subtreeFlags traversal.
-        //
-        // A reference to the deletion array of the parent is also stored on
-        // each of the deletions. This is really weird. It would be better to
-        // follow the `.return` pointer, but unfortunately we can't assume that
-        // `.return` points to the correct fiber, even in the commit phase,
-        // because `findDOMNode` might mutate it.
-        const deletedChild = nextEffect;
-        const deletions = deletedChild.deletions;
-        if (deletions !== null) {
-          for (let i = 0; i < deletions.length; i++) {
-            const deletion = deletions[i];
-            // Clear the deletion effect so that we don't delete this node more
-            // than once.
-            deletion.flags &= ~Deletion;
-            deletion.deletions = null;
-            commitDeletion(root, deletion, renderPriorityLevel);
-          }
-        }
-        break;
-      }
-    }
-
-    resetCurrentDebugFiberInDEV();
-    nextEffect = nextEffect.nextEffect;
-  }
-}
-
 export function flushPassiveEffects(): boolean {
   // Returns whether passive effects were flushed.
   if (pendingPassiveEffectsRenderPriority !== NoSchedulerPriority) {