Repository: facebook/react
Pull request: Pass lanes as argument to performSyncWorkOnRoot

`performSyncWorkOnRoot` has only a single caller, and the caller already computes the next lanes (`getNextLanes`) before deciding to call the function. So we can pass them as an argument instead of computing the lanes again.

There was already a TODO comment about this, but it was mostly perf related. However, @rickhanlonii noticed a discrepancy where the inner `getNextLanes` call was not being passed the current work-in- progress lanes. Usually this shouldn't matter because there should never be work-in-progress sync work; it should finish immediately. There is one case I'm aware of where we exit the work loop without finishing a sync render, which is selective hydration, but even then it should switch to the sync hydration lane, not the normal sync lane. So something else is probably going on. I suspect it might be related to the `enableUnifiedSyncLane` experiment.

This is likely related to a regression found internally at Meta. We're still working on getting a proper regression test; I can come up with a contrived one but I'm not confident it'll be the same as the actual regression until we get a better repro.

Changed files:
filename: packages/react-reconciler/src/ReactFiberRootScheduler.js
patch: @@ -164,9 +164,6 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
     return;
   }
 
-  const workInProgressRoot = getWorkInProgressRoot();
-  const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes();
-
   // There may or may not be synchronous work scheduled. Let's check.
   let didPerformSomeWork;
   let errors: Array<mixed> | null = null;
@@ -178,17 +175,18 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
       if (onlyLegacy && root.tag !== LegacyRoot) {
         // Skip non-legacy roots.
       } else {
+        const workInProgressRoot = getWorkInProgressRoot();
+        const workInProgressRootRenderLanes =
+          getWorkInProgressRootRenderLanes();
         const nextLanes = getNextLanes(
           root,
           root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,
         );
         if (includesSyncLane(nextLanes)) {
           // This root has pending sync work. Flush it now.
           try {
-            // TODO: Pass nextLanes as an argument instead of computing it again
-            // inside performSyncWorkOnRoot.
             didPerformSomeWork = true;
-            performSyncWorkOnRoot(root);
+            performSyncWorkOnRoot(root, nextLanes);
           } catch (error) {
             // Collect errors so we can rethrow them at the end
             if (errors === null) {

filename: packages/react-reconciler/src/ReactFiberWorkLoop.js
patch: @@ -1254,25 +1254,27 @@ function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
 
 // This is the entry point for synchronous tasks that don't go
 // through Scheduler
-export function performSyncWorkOnRoot(root: FiberRoot): null {
-  if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
-    syncNestedUpdateFlag();
-  }
-
+export function performSyncWorkOnRoot(root: FiberRoot, lanes: Lanes): null {
   if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
     throw new Error('Should not already be working.');
   }
 
-  flushPassiveEffects();
-
-  // TODO: This was already computed in the caller. Pass it as an argument.
-  let lanes = getNextLanes(root, NoLanes);
-  if (!includesSyncLane(lanes)) {
-    // There's no remaining sync work left.
+  const didFlushPassiveEffects = flushPassiveEffects();
+  if (didFlushPassiveEffects) {
+    // If passive effects were flushed, exit to the outer work loop in the root
+    // scheduler, so we can recompute the priority.
+    // TODO: We don't actually need this `ensureRootIsScheduled` call because
+    // this path is only reachable if the root is already part of the schedule.
+    // I'm including it only for consistency with the other exit points from
+    // this function. Can address in a subsequent refactor.
     ensureRootIsScheduled(root);
     return null;
   }
 
+  if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
+    syncNestedUpdateFlag();
+  }
+
   let exitStatus = renderRootSync(root, lanes);
   if (root.tag !== LegacyRoot && exitStatus === RootErrored) {
     // If something threw an error, try rendering one more time. We'll render