Bugfix: Fix race condition between interleaved and non-interleaved updates

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

@@ -28,6 +28,10 @@ export function pushInterleavedQueue(
   }
 }
 
+export function hasInterleavedUpdates() {
+  return interleavedQueues !== null;
+}
+
 export function enqueueInterleavedUpdates() {
   // Transfer the interleaved updates onto the main queue. Each queue has a
   // `pending` field and an `interleaved` field. When they are not null, they

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

@@ -28,6 +28,10 @@ export function pushInterleavedQueue(
   }
 }
 
+export function hasInterleavedUpdates() {
+  return interleavedQueues !== null;
+}
+
 export function enqueueInterleavedUpdates() {
   // Transfer the interleaved updates onto the main queue. Each queue has a
   // `pending` field and an `interleaved` field. When they are not null, they

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

@@ -195,7 +195,10 @@ import {
   pop as popFromStack,
   createCursor,
 } from './ReactFiberStack.new';
-import {enqueueInterleavedUpdates} from './ReactFiberInterleavedUpdates.new';
+import {
+  enqueueInterleavedUpdates,
+  hasInterleavedUpdates,
+} from './ReactFiberInterleavedUpdates.new';
 
 import {
   markNestedUpdateScheduled,
@@ -723,7 +726,13 @@ export function isInterleavedUpdate(fiber: Fiber, lane: Lane) {
     // TODO: Optimize slightly by comparing to root that fiber belongs to.
     // Requires some refactoring. Not a big deal though since it's rare for
     // concurrent apps to have more than a single root.
-    workInProgressRoot !== null &&
+    (workInProgressRoot !== null ||
+      // If the interleaved updates queue hasn't been cleared yet, then
+      // we should treat this as an interleaved update, too. This is also a
+      // defensive coding measure in case a new update comes in between when
+      // rendering has finished and when the interleaved updates are transferred
+      // to the main queue.
+      hasInterleavedUpdates() !== null) &&
     (fiber.mode & ConcurrentMode) !== NoMode &&
     // If this is a render phase update (i.e. UNSAFE_componentWillReceiveProps),
     // then don't treat this as an interleaved update. This pattern is

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

@@ -195,7 +195,10 @@ import {
   pop as popFromStack,
   createCursor,
 } from './ReactFiberStack.old';
-import {enqueueInterleavedUpdates} from './ReactFiberInterleavedUpdates.old';
+import {
+  enqueueInterleavedUpdates,
+  hasInterleavedUpdates,
+} from './ReactFiberInterleavedUpdates.old';
 
 import {
   markNestedUpdateScheduled,
@@ -723,7 +726,13 @@ export function isInterleavedUpdate(fiber: Fiber, lane: Lane) {
     // TODO: Optimize slightly by comparing to root that fiber belongs to.
     // Requires some refactoring. Not a big deal though since it's rare for
     // concurrent apps to have more than a single root.
-    workInProgressRoot !== null &&
+    (workInProgressRoot !== null ||
+      // If the interleaved updates queue hasn't been cleared yet, then
+      // we should treat this as an interleaved update, too. This is also a
+      // defensive coding measure in case a new update comes in between when
+      // rendering has finished and when the interleaved updates are transferred
+      // to the main queue.
+      hasInterleavedUpdates() !== null) &&
     (fiber.mode & ConcurrentMode) !== NoMode &&
     // If this is a render phase update (i.e. UNSAFE_componentWillReceiveProps),
     // then don't treat this as an interleaved update. This pattern is

packages/react-reconciler/src/__tests__/ReactInterleavedUpdates-test.js

@@ -140,4 +140,52 @@ describe('ReactInterleavedUpdates', () => {
     expect(Scheduler).toHaveYielded([2, 2, 2]);
     expect(root).toMatchRenderedOutput('222');
   });
+
+  test('regression for #24350: does not add to main update queue until interleaved update queue has been cleared', async () => {
+    let setStep;
+    function App() {
+      const [step, _setState] = useState(0);
+      setStep = _setState;
+      return (
+        <>
+          <Text text={'A' + step} />
+          <Text text={'B' + step} />
+          <Text text={'C' + step} />
+        </>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(<App />);
+    });
+    expect(Scheduler).toHaveYielded(['A0', 'B0', 'C0']);
+    expect(root).toMatchRenderedOutput('A0B0C0');
+
+    await act(async () => {
+      // Start the render phase.
+      startTransition(() => {
+        setStep(1);
+      });
+      expect(Scheduler).toFlushAndYieldThrough(['A1', 'B1']);
+
+      // Schedule an interleaved update. This gets placed on a special queue.
+      startTransition(() => {
+        setStep(2);
+      });
+
+      // Finish rendering the first update.
+      expect(Scheduler).toFlushUntilNextPaint(['C1']);
+
+      // Schedule another update. (In the regression case, this was treated
+      // as a normal, non-interleaved update and it was inserted into the queue
+      // before the interleaved one was processed.)
+      startTransition(() => {
+        setStep(3);
+      });
+    });
+    // The last update should win.
+    expect(Scheduler).toHaveYielded(['A3', 'B3', 'C3']);
+    expect(root).toMatchRenderedOutput('A3B3C3');
+  });
 });