Remove Passive flag from "before mutation" phase

We don't need to visit passive effect nodes during before mutation. The only reason we were previously was to schedule the root-level passive effect callback as early as possible, but now that `subtreeFlags` exists, we can check that instead. This should reduce the amount of traversal during the commit phase, particularly when mounting or updating large trees that contain many passive effects.
facebook · Oct 15, 2020 · df06c46 · df06c46
1 parent c57fe4a
commit df06c46
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 27 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js
@@ -48,7 +48,10 @@ export const ForceUpdateForLegacySuspense = /* */ 0b000100000000000000;
 export const PassiveStatic = /*                */ 0b001000000000000000;
 
 // Union of side effect groupings as pertains to subtreeFlags
-export const BeforeMutationMask = /*           */ 0b000000001100001010;
+// TODO: Don't need to visit Placement during BeforeMutation phase
+// TODO: Only need to visit Deletions during BeforeMutation phase if an element
+// is focused.
+export const BeforeMutationMask = /*           */ 0b000000000100001010;
 export const MutationMask = /*                 */ 0b000000010010011110;
 export const LayoutMask = /*                   */ 0b000000000010100100;
 export const PassiveMask = /*                  */ 0b000000001000001000;

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -1880,6 +1880,20 @@ function commitRootImpl(root, renderPriorityLevel) {
     NoFlags;
 
   if (subtreeHasEffects || rootHasEffect) {
+    // If there are pending passive effects, schedule a callback to process them.
+    if (
+      (finishedWork.subtreeFlags & PassiveMask) !== NoFlags ||
+      (finishedWork.flags & PassiveMask) !== NoFlags
+    ) {
+      if (!rootDoesHavePassiveEffects) {
+        rootDoesHavePassiveEffects = true;
+        scheduleCallback(NormalSchedulerPriority, () => {
+          flushPassiveEffects();
+          return null;
+        });
+      }
+    }
+
     let previousLanePriority;
     if (decoupleUpdatePriorityFromScheduler) {
       previousLanePriority = getCurrentUpdateLanePriority();
@@ -1972,20 +1986,6 @@ function commitRootImpl(root, renderPriorityLevel) {
       markLayoutEffectsStopped();
     }
 
-    // If there are pending passive effects, schedule a callback to process them.
-    if (
-      (finishedWork.subtreeFlags & PassiveMask) !== NoFlags ||
-      (finishedWork.flags & PassiveMask) !== NoFlags
-    ) {
-      if (!rootDoesHavePassiveEffects) {
-        rootDoesHavePassiveEffects = true;
-        scheduleCallback(NormalSchedulerPriority, () => {
-          flushPassiveEffects();
-          return null;
-        });
-      }
-    }
-
     // Tell Scheduler to yield at the end of the frame, so the browser has an
     // opportunity to paint.
     requestPaint();
@@ -2181,18 +2181,6 @@ function commitBeforeMutationEffectsImpl(fiber: Fiber) {
     commitBeforeMutationEffectOnFiber(current, fiber);
     resetCurrentDebugFiberInDEV();
   }
-
-  if ((flags & Passive) !== NoFlags) {
-    // If there are passive effects, schedule a callback to flush at
-    // the earliest opportunity.
-    if (!rootDoesHavePassiveEffects) {
-      rootDoesHavePassiveEffects = true;
-      scheduleCallback(NormalSchedulerPriority, () => {
-        flushPassiveEffects();
-        return null;
-      });
-    }
-  }
 }
 
 function commitBeforeMutationEffectsDeletions(deletions: Array<Fiber>) {