Don't shift interleaved updates to separate lane (#20681)

Now that interleaved updates are added to a special queue, we no longer
need to shift them into their own lane. We can add to a lane that's
already in the middle of rendering without risk of tearing.

See #20615 for more background.

I've only changed this in the new fork, and only behind the
enableTransitionEntanglements flag.

Most of this commit involves updating tests. The "shift-to-a-new" lane
trick was intentionally used in a handful of tests where two or more
updates need to be scheduled in different lanes. Most of these tests
were written before `startTransition` existed, and all of them were
written before transitions were assigned arbitrary lanes.

So I ported these tests to use `startTransition` instead, which is the
idiomatic way to mark an update as parallel.

I didn't change the old fork at all. Writing these tests in such a way
that they also pass in the old fork actually revealed a few flaws in the
current implementation regarding interrupting a suspended refresh
transition early, which is a good reminder that we should be writing our
tests using idiomatic patterns as much as we possibly can.

Author: acdlite

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

@@ -2029,10 +2029,11 @@ function dispatchAction<S, A>(
 
       // Entangle the new transition lane with the other transition lanes.
       const newQueueLanes = mergeLanes(queueLanes, lane);
-      if (newQueueLanes !== queueLanes) {
-        queue.lanes = newQueueLanes;
-        markRootEntangled(root, newQueueLanes);
-      }
+      queue.lanes = newQueueLanes;
+      // Even if queue.lanes already include lane, we don't know for certain if
+      // the lane finished since the last time we entangled it. So we need to
+      // entangle it again, just to be sure.
+      markRootEntangled(root, newQueueLanes);
     }
   }
 

packages/react-reconciler/src/ReactFiberHooks.old.js

@@ -2029,10 +2029,11 @@ function dispatchAction<S, A>(
 
       // Entangle the new transition lane with the other transition lanes.
       const newQueueLanes = mergeLanes(queueLanes, lane);
-      if (newQueueLanes !== queueLanes) {
-        queue.lanes = newQueueLanes;
-        markRootEntangled(root, newQueueLanes);
-      }
+      queue.lanes = newQueueLanes;
+      // Even if queue.lanes already include lane, we don't know for certain if
+      // the lane finished since the last time we entangled it. So we need to
+      // entangle it again, just to be sure.
+      markRootEntangled(root, newQueueLanes);
     }
   }
 

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

@@ -36,7 +36,10 @@ export type Lane = number;
 export type LaneMap<T> = Array<T>;
 
 import invariant from 'shared/invariant';
-import {enableCache} from 'shared/ReactFeatureFlags';
+import {
+  enableCache,
+  enableTransitionEntanglement,
+} from 'shared/ReactFeatureFlags';
 
 import {
   ImmediatePriority as ImmediateSchedulerPriority,
@@ -92,11 +95,12 @@ export const DefaultLanes: Lanes = /*                   */ 0b0000000000000000000
 
 const TransitionHydrationLane: Lane = /*                */ 0b0000000000000000001000000000000;
 const TransitionLanes: Lanes = /*                       */ 0b0000000001111111110000000000000;
-const SomeTransitionLane: Lane = /*                     */ 0b0000000000000000010000000000000;
+const FirstTransitionLane: Lane = /*                    */ 0b0000000000000000010000000000000;
 
 const RetryLanes: Lanes = /*                            */ 0b0000011110000000000000000000000;
 
-export const SomeRetryLane: Lanes = /*                  */ 0b0000010000000000000000000000000;
+const FirstRetryLane: Lanes = /*                        */ 0b0000000010000000000000000000000;
+export const SomeRetryLane: Lane = FirstRetryLane;
 
 export const SelectiveHydrationLane: Lane = /*          */ 0b0000100000000000000000000000000;
 
@@ -111,8 +115,8 @@ export const NoTimestamp = -1;
 
 let currentUpdateLanePriority: LanePriority = NoLanePriority;
 
-let nextTransitionLane: Lane = SomeTransitionLane;
-let nextRetryLane: Lane = SomeRetryLane;
+let nextTransitionLane: Lane = FirstTransitionLane;
+let nextRetryLane: Lane = FirstRetryLane;
 
 export function getCurrentUpdateLanePriority(): LanePriority {
   return currentUpdateLanePriority;
@@ -498,56 +502,88 @@ export function findUpdateLane(
   lanePriority: LanePriority,
   wipLanes: Lanes,
 ): Lane {
-  switch (lanePriority) {
-    case NoLanePriority:
-      break;
-    case SyncLanePriority:
-      return SyncLane;
-    case SyncBatchedLanePriority:
-      return SyncBatchedLane;
-    case InputDiscreteLanePriority: {
-      const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
-      if (lane === NoLane) {
-        // Shift to the next priority level
-        return findUpdateLane(InputContinuousLanePriority, wipLanes);
+  if (enableTransitionEntanglement) {
+    // Ignore wipLanes. Always assign to the same bit per priority.
+    switch (lanePriority) {
+      case NoLanePriority:
+        break;
+      case SyncLanePriority:
+        return SyncLane;
+      case SyncBatchedLanePriority:
+        return SyncBatchedLane;
+      case InputDiscreteLanePriority: {
+        return pickArbitraryLane(InputDiscreteLanes);
       }
-      return lane;
-    }
-    case InputContinuousLanePriority: {
-      const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
-      if (lane === NoLane) {
-        // Shift to the next priority level
-        return findUpdateLane(DefaultLanePriority, wipLanes);
+      case InputContinuousLanePriority: {
+        return pickArbitraryLane(InputContinuousLanes);
+      }
+      case DefaultLanePriority: {
+        return pickArbitraryLane(DefaultLanes);
       }
-      return lane;
+      case TransitionPriority: // Should be handled by findTransitionLane instead
+      case RetryLanePriority: // Should be handled by findRetryLane instead
+        break;
+      case IdleLanePriority:
+        return pickArbitraryLane(IdleLanes);
+      default:
+        // The remaining priorities are not valid for updates
+        break;
     }
-    case DefaultLanePriority: {
-      let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
-      if (lane === NoLane) {
-        // If all the default lanes are already being worked on, look for a
-        // lane in the transition range.
-        lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
+  } else {
+    // Old behavior that uses wipLanes to shift interleaved updates into a
+    // separate lane. This is no longer needed because we put interleaved
+    // updates on a special queue.
+    switch (lanePriority) {
+      case NoLanePriority:
+        break;
+      case SyncLanePriority:
+        return SyncLane;
+      case SyncBatchedLanePriority:
+        return SyncBatchedLane;
+      case InputDiscreteLanePriority: {
+        const lane = pickArbitraryLane(InputDiscreteLanes & ~wipLanes);
         if (lane === NoLane) {
-          // All the transition lanes are taken, too. This should be very
-          // rare, but as a last resort, pick a default lane. This will have
-          // the effect of interrupting the current work-in-progress render.
-          lane = pickArbitraryLane(DefaultLanes);
+          // Shift to the next priority level
+          return findUpdateLane(InputContinuousLanePriority, wipLanes);
         }
+        return lane;
       }
-      return lane;
-    }
-    case TransitionPriority: // Should be handled by findTransitionLane instead
-    case RetryLanePriority: // Should be handled by findRetryLane instead
-      break;
-    case IdleLanePriority:
-      let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
-      if (lane === NoLane) {
-        lane = pickArbitraryLane(IdleLanes);
+      case InputContinuousLanePriority: {
+        const lane = pickArbitraryLane(InputContinuousLanes & ~wipLanes);
+        if (lane === NoLane) {
+          // Shift to the next priority level
+          return findUpdateLane(DefaultLanePriority, wipLanes);
+        }
+        return lane;
       }
-      return lane;
-    default:
-      // The remaining priorities are not valid for updates
-      break;
+      case DefaultLanePriority: {
+        let lane = pickArbitraryLane(DefaultLanes & ~wipLanes);
+        if (lane === NoLane) {
+          // If all the default lanes are already being worked on, look for a
+          // lane in the transition range.
+          lane = pickArbitraryLane(TransitionLanes & ~wipLanes);
+          if (lane === NoLane) {
+            // All the transition lanes are taken, too. This should be very
+            // rare, but as a last resort, pick a default lane. This will have
+            // the effect of interrupting the current work-in-progress render.
+            lane = pickArbitraryLane(DefaultLanes);
+          }
+        }
+        return lane;
+      }
+      case TransitionPriority: // Should be handled by findTransitionLane instead
+      case RetryLanePriority: // Should be handled by findRetryLane instead
+        break;
+      case IdleLanePriority:
+        let lane = pickArbitraryLane(IdleLanes & ~wipLanes);
+        if (lane === NoLane) {
+          lane = pickArbitraryLane(IdleLanes);
+        }
+        return lane;
+      default:
+        // The remaining priorities are not valid for updates
+        break;
+    }
   }
   invariant(
     false,
@@ -563,7 +599,7 @@ export function claimNextTransitionLane(): Lane {
   const lane = nextTransitionLane;
   nextTransitionLane <<= 1;
   if ((nextTransitionLane & TransitionLanes) === 0) {
-    nextTransitionLane = SomeTransitionLane;
+    nextTransitionLane = FirstTransitionLane;
   }
   return lane;
 }
@@ -572,7 +608,7 @@ export function claimNextRetryLane(): Lane {
   const lane = nextRetryLane;
   nextRetryLane <<= 1;
   if ((nextRetryLane & RetryLanes) === 0) {
-    nextRetryLane = SomeRetryLane;
+    nextRetryLane = FirstRetryLane;
   }
   return lane;
 }