Don't shift interleaved updates to separate lane

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 facebook#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/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,
@@ -498,56 +501,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,

packages/react-reconciler/src/ReactFiberLane.old.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,
@@ -498,56 +501,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,

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

@@ -34,6 +34,7 @@ import {
   disableSchedulerTimeoutInWorkLoop,
   enableDoubleInvokingEffects,
   skipUnmountedBoundaries,
+  enableTransitionEntanglement,
 } from 'shared/ReactFeatureFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import invariant from 'shared/invariant';
@@ -832,6 +833,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
   let exitStatus = renderRootConcurrent(root, lanes);
 
   if (
+    !enableTransitionEntanglement &&
     includesSomeLane(
       workInProgressRootIncludedLanes,
       workInProgressRootUpdatedLanes,
@@ -1037,6 +1039,7 @@ function performSyncWorkOnRoot(root) {
     lanes = workInProgressRootRenderLanes;
     exitStatus = renderRootSync(root, lanes);
     if (
+      !enableTransitionEntanglement &&
       includesSomeLane(
         workInProgressRootIncludedLanes,
         workInProgressRootUpdatedLanes,

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

@@ -34,6 +34,7 @@ import {
   disableSchedulerTimeoutInWorkLoop,
   enableDoubleInvokingEffects,
   skipUnmountedBoundaries,
+  enableTransitionEntanglement,
 } from 'shared/ReactFeatureFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import invariant from 'shared/invariant';
@@ -832,6 +833,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
   let exitStatus = renderRootConcurrent(root, lanes);
 
   if (
+    !enableTransitionEntanglement &&
     includesSomeLane(
       workInProgressRootIncludedLanes,
       workInProgressRootUpdatedLanes,
@@ -1037,6 +1039,7 @@ function performSyncWorkOnRoot(root) {
     lanes = workInProgressRootRenderLanes;
     exitStatus = renderRootSync(root, lanes);
     if (
+      !enableTransitionEntanglement &&
       includesSomeLane(
         workInProgressRootIncludedLanes,
         workInProgressRootUpdatedLanes,

packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js

@@ -327,6 +327,9 @@ describe('DebugTracing', () => {
     ]);
   });
 
+  // This test is coupled to lane implementation details, so I'm disabling it in
+  // the new fork until it stabilizes so we don't have to repeatedly update it.
+  // @gate !enableTransitionEntanglement
   // @gate experimental && build === 'development' && enableDebugTracing
   it('should log cascading passive updates', () => {
     function Example() {