Don't update childLanes until after current render

(This is the riskiest commit in the stack.)

Updates that occur in a concurrent event while a render is already in
progress can't be processed during that render. This is tricky to get
right. Previously we solved this by adding concurrent updates to a
special `interleaved` queue, then transferring the `interleaved` queue
to the `pending` queue after the render phase had completed.

However, we would still mutate the `childLanes` along the parent path
immediately, which can lead to its own subtle data races.

Instead, we can queue the entire operation until after the render phase
has completed. This replaces the need for an `interleaved` field on
every fiber/hook queue.

The main motivation for this change, aside from simplifying the logic a
bit, is so we can read information about the current fiber while we're
walking up its return path, like whether it's inside a hidden tree.
(I haven't done anything like that in this commit, though.)

Author: acdlite

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

@@ -132,7 +132,6 @@ export type Update<State> = {|
 
 export type SharedQueue<State> = {|
   pending: Update<State> | null,
-  interleaved: Update<State> | null,
   lanes: Lanes,
 |};
 
@@ -172,7 +171,6 @@ export function initializeUpdateQueue<State>(fiber: Fiber): void {
     lastBaseUpdate: null,
     shared: {
       pending: null,
-      interleaved: null,
       lanes: NoLanes,
     },
     effects: null,
@@ -622,17 +620,7 @@ export function processUpdateQueue<State>(
     queue.firstBaseUpdate = newFirstBaseUpdate;
     queue.lastBaseUpdate = newLastBaseUpdate;
 
-    // Interleaved updates are stored on a separate queue. We aren't going to
-    // process them during this render, but we do need to track which lanes
-    // are remaining.
-    const lastInterleaved = queue.shared.interleaved;
-    if (lastInterleaved !== null) {
-      let interleaved = lastInterleaved;
-      do {
-        newLanes = mergeLanes(newLanes, interleaved.lane);
-        interleaved = ((interleaved: any).next: Update<State>);
-      } while (interleaved !== lastInterleaved);
-    } else if (firstBaseUpdate === null) {
+    if (firstBaseUpdate === null) {
       // `queue.lanes` is used for entangling transitions. We can set it back to
       // zero once the queue is empty.
       queue.shared.lanes = NoLanes;

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

@@ -132,7 +132,6 @@ export type Update<State> = {|
 
 export type SharedQueue<State> = {|
   pending: Update<State> | null,
-  interleaved: Update<State> | null,
   lanes: Lanes,
 |};
 
@@ -172,7 +171,6 @@ export function initializeUpdateQueue<State>(fiber: Fiber): void {
     lastBaseUpdate: null,
     shared: {
       pending: null,
-      interleaved: null,
       lanes: NoLanes,
     },
     effects: null,
@@ -622,17 +620,7 @@ export function processUpdateQueue<State>(
     queue.firstBaseUpdate = newFirstBaseUpdate;
     queue.lastBaseUpdate = newLastBaseUpdate;
 
-    // Interleaved updates are stored on a separate queue. We aren't going to
-    // process them during this render, but we do need to track which lanes
-    // are remaining.
-    const lastInterleaved = queue.shared.interleaved;
-    if (lastInterleaved !== null) {
-      let interleaved = lastInterleaved;
-      do {
-        newLanes = mergeLanes(newLanes, interleaved.lane);
-        interleaved = ((interleaved: any).next: Update<State>);
-      } while (interleaved !== lastInterleaved);
-    } else if (firstBaseUpdate === null) {
+    if (firstBaseUpdate === null) {
       // `queue.lanes` is used for entangling transitions. We can set it back to
       // zero once the queue is empty.
       queue.shared.lanes = NoLanes;

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

@@ -16,169 +16,208 @@ import type {
   SharedQueue as ClassQueue,
   Update as ClassUpdate,
 } from './ReactFiberClassUpdateQueue.new';
-import type {Lane} from './ReactFiberLane.new';
+import type {Lane, Lanes} from './ReactFiberLane.new';
 
 import {warnAboutUpdateOnNotYetMountedFiberInDEV} from './ReactFiberWorkLoop.new';
-import {mergeLanes} from './ReactFiberLane.new';
+import {
+  NoLane,
+  NoLanes,
+  mergeLanes,
+  isSubsetOfLanes,
+} from './ReactFiberLane.new';
 import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
 import {HostRoot} from './ReactWorkTags';
 
-// An array of all update queues that received updates during the current
-// render. When this render exits, either because it finishes or because it is
-// interrupted, the interleaved updates will be transferred onto the main part
-// of the queue.
-let concurrentQueues: Array<
-  HookQueue<any, any> | ClassQueue<any>,
-> | null = null;
+type ConcurrentUpdate = {
+  next: ConcurrentUpdate,
+};
 
-export function pushConcurrentUpdateQueue(
-  queue: HookQueue<any, any> | ClassQueue<any>,
-) {
-  if (concurrentQueues === null) {
-    concurrentQueues = [queue];
-  } else {
-    concurrentQueues.push(queue);
-  }
-}
+type ConcurrentQueue = {
+  pending: ConcurrentUpdate | null,
+};
+
+// If a render is in progress, and we receive an update from a concurrent event,
+// we wait until the current render is over (either finished or interrupted)
+// before adding it to the fiber/hook queue. Push to this array so we can
+// access the queue, fiber, update, et al later.
+const concurrentQueues: Array<any> = [];
+let concurrentQueuesIndex = 0;
 
-export function finishQueueingConcurrentUpdates() {
-  // Transfer the interleaved updates onto the main queue. Each queue has a
-  // `pending` field and an `interleaved` field. When they are not null, they
-  // point to the last node in a circular linked list. We need to append the
-  // interleaved list to the end of the pending list by joining them into a
-  // single, circular list.
-  if (concurrentQueues !== null) {
-    for (let i = 0; i < concurrentQueues.length; i++) {
-      const queue = concurrentQueues[i];
-      const lastInterleavedUpdate = queue.interleaved;
-      if (lastInterleavedUpdate !== null) {
-        queue.interleaved = null;
-        const firstInterleavedUpdate = lastInterleavedUpdate.next;
-        const lastPendingUpdate = queue.pending;
-        if (lastPendingUpdate !== null) {
-          const firstPendingUpdate = lastPendingUpdate.next;
-          lastPendingUpdate.next = (firstInterleavedUpdate: any);
-          lastInterleavedUpdate.next = (firstPendingUpdate: any);
-        }
-        queue.pending = (lastInterleavedUpdate: any);
+export function finishQueueingConcurrentUpdates(): Lanes {
+  const endIndex = concurrentQueuesIndex;
+  concurrentQueuesIndex = 0;
+
+  let lanes = NoLanes;
+
+  let i = 0;
+  while (i < endIndex) {
+    const fiber: Fiber = concurrentQueues[i];
+    concurrentQueues[i++] = null;
+    const queue: ConcurrentQueue = concurrentQueues[i];
+    concurrentQueues[i++] = null;
+    const update: ConcurrentUpdate = concurrentQueues[i];
+    concurrentQueues[i++] = null;
+    const lane: Lane = concurrentQueues[i];
+    concurrentQueues[i++] = null;
+
+    if (queue !== null && update !== null) {
+      const pending = queue.pending;
+      if (pending === null) {
+        // This is the first update. Create a circular list.
+        update.next = update;
+      } else {
+        update.next = pending.next;
+        pending.next = update;
       }
+      queue.pending = update;
+    }
+
+    if (lane !== NoLane) {
+      lanes = mergeLanes(lanes, lane);
+      markUpdateLaneFromFiberToRoot(fiber, lane);
     }
-    concurrentQueues = null;
   }
+
+  return lanes;
 }
 
-export function enqueueConcurrentHookUpdate<S, A>(
+function enqueueUpdate(
   fiber: Fiber,
-  queue: HookQueue<S, A>,
-  update: HookUpdate<S, A>,
+  queue: ConcurrentQueue | null,
+  update: ConcurrentUpdate | null,
   lane: Lane,
 ) {
-  const interleaved = queue.interleaved;
-  if (interleaved === null) {
-    // This is the first update. Create a circular list.
-    update.next = update;
-    // At the end of the current render, this queue's interleaved updates will
-    // be transferred to the pending queue.
-    pushConcurrentUpdateQueue(queue);
-  } else {
-    update.next = interleaved.next;
-    interleaved.next = update;
+  // Don't update the `childLanes` on the return path yet. If we already in
+  // the middle of rendering, wait until after it has completed.
+  concurrentQueues[concurrentQueuesIndex++] = fiber;
+  concurrentQueues[concurrentQueuesIndex++] = queue;
+  concurrentQueues[concurrentQueuesIndex++] = update;
+  concurrentQueues[concurrentQueuesIndex++] = lane;
+
+  // The fiber's `lane` field is used in some places to check if any work is
+  // scheduled, to perform an eager bailout, so we need to update it immediately.
+  // TODO: We should probably move this to the "shared" queue instead.
+  fiber.lanes = mergeLanes(fiber.lanes, lane);
+  const alternate = fiber.alternate;
+  if (alternate !== null) {
+    alternate.lanes = mergeLanes(alternate.lanes, lane);
   }
-  queue.interleaved = update;
+}
 
-  return markUpdateLaneFromFiberToRoot(fiber, lane);
+export function enqueueConcurrentHookUpdate<S, A>(
+  fiber: Fiber,
+  queue: HookQueue<S, A>,
+  update: HookUpdate<S, A>,
+  lane: Lane,
+): FiberRoot | null {
+  const concurrentQueue: ConcurrentQueue = (queue: any);
+  const concurrentUpdate: ConcurrentUpdate = (update: any);
+  enqueueUpdate(fiber, concurrentQueue, concurrentUpdate, lane);
+  return getRootForUpdatedFiber(fiber);
 }
 
 export function enqueueConcurrentHookUpdateAndEagerlyBailout<S, A>(
   fiber: Fiber,
   queue: HookQueue<S, A>,
   update: HookUpdate<S, A>,
-  lane: Lane,
 ): void {
-  const interleaved = queue.interleaved;
-  if (interleaved === null) {
-    // This is the first update. Create a circular list.
-    update.next = update;
-    // At the end of the current render, this queue's interleaved updates will
-    // be transferred to the pending queue.
-    pushConcurrentUpdateQueue(queue);
-  } else {
-    update.next = interleaved.next;
-    interleaved.next = update;
-  }
-  queue.interleaved = update;
+  // This function is used to queue an update that doesn't need a rerender. The
+  // only reason we queue it is in case there's a subsequent higher priority
+  // update that causes it to be rebased.
+  const lane = NoLane;
+  const concurrentQueue: ConcurrentQueue = (queue: any);
+  const concurrentUpdate: ConcurrentUpdate = (update: any);
+  enqueueUpdate(fiber, concurrentQueue, concurrentUpdate, lane);
 }
 
 export function enqueueConcurrentClassUpdate<State>(
   fiber: Fiber,
   queue: ClassQueue<State>,
   update: ClassUpdate<State>,
   lane: Lane,
-) {
-  const interleaved = queue.interleaved;
-  if (interleaved === null) {
-    // This is the first update. Create a circular list.
-    update.next = update;
-    // At the end of the current render, this queue's interleaved updates will
-    // be transferred to the pending queue.
-    pushConcurrentUpdateQueue(queue);
-  } else {
-    update.next = interleaved.next;
-    interleaved.next = update;
-  }
-  queue.interleaved = update;
-
-  return markUpdateLaneFromFiberToRoot(fiber, lane);
+): FiberRoot | null {
+  const concurrentQueue: ConcurrentQueue = (queue: any);
+  const concurrentUpdate: ConcurrentUpdate = (update: any);
+  enqueueUpdate(fiber, concurrentQueue, concurrentUpdate, lane);
+  return getRootForUpdatedFiber(fiber);
 }
 
-export function enqueueConcurrentRenderForLane(fiber: Fiber, lane: Lane) {
-  return markUpdateLaneFromFiberToRoot(fiber, lane);
+export function enqueueConcurrentRenderForLane(
+  fiber: Fiber,
+  lane: Lane,
+): FiberRoot | null {
+  enqueueUpdate(fiber, null, null, lane);
+  return getRootForUpdatedFiber(fiber);
 }
 
 // Calling this function outside this module should only be done for backwards
 // compatibility and should always be accompanied by a warning.
-export const unsafe_markUpdateLaneFromFiberToRoot = markUpdateLaneFromFiberToRoot;
-
-function markUpdateLaneFromFiberToRoot(
+export function unsafe_markUpdateLaneFromFiberToRoot(
   sourceFiber: Fiber,
   lane: Lane,
 ): FiberRoot | null {
+  markUpdateLaneFromFiberToRoot(sourceFiber, lane);
+  return getRootForUpdatedFiber(sourceFiber);
+}
+
+function markUpdateLaneFromFiberToRoot(sourceFiber: Fiber, lane: Lane): void {
   // Update the source fiber's lanes
   sourceFiber.lanes = mergeLanes(sourceFiber.lanes, lane);
   let alternate = sourceFiber.alternate;
   if (alternate !== null) {
     alternate.lanes = mergeLanes(alternate.lanes, lane);
   }
-  if (__DEV__) {
-    if (
-      alternate === null &&
-      (sourceFiber.flags & (Placement | Hydrating)) !== NoFlags
-    ) {
-      warnAboutUpdateOnNotYetMountedFiberInDEV(sourceFiber);
-    }
-  }
   // Walk the parent path to the root and update the child lanes.
-  let node = sourceFiber;
   let parent = sourceFiber.return;
   while (parent !== null) {
-    parent.childLanes = mergeLanes(parent.childLanes, lane);
     alternate = parent.alternate;
-    if (alternate !== null) {
-      alternate.childLanes = mergeLanes(alternate.childLanes, lane);
+    if (isSubsetOfLanes(parent.childLanes, lane)) {
+      if (alternate === null || isSubsetOfLanes(alternate.childLanes, lane)) {
+        // Both fibers already have sufficient priority. Don't need to update
+        // the rest of the return path. This is a helpful optimization in the
+        // case where the same component is updated many times in rapid
+        // succession, or even in the same event.
+        return;
+      } else {
+        alternate.childLanes = mergeLanes(alternate.childLanes, lane);
+      }
     } else {
-      if (__DEV__) {
-        if ((parent.flags & (Placement | Hydrating)) !== NoFlags) {
-          warnAboutUpdateOnNotYetMountedFiberInDEV(sourceFiber);
-        }
+      parent.childLanes = mergeLanes(parent.childLanes, lane);
+      if (alternate !== null) {
+        alternate.childLanes = mergeLanes(alternate.childLanes, lane);
       }
     }
-    node = parent;
     parent = parent.return;
   }
-  if (node.tag === HostRoot) {
-    const root: FiberRoot = node.stateNode;
-    return root;
-  } else {
-    return null;
+}
+
+function getRootForUpdatedFiber(sourceFiber: Fiber): FiberRoot | null {
+  // When a setState happens, we must ensure the root is scheduled. Because
+  // update queues do not have a backpointer to the root, the only way to do
+  // this currently is to walk up the return path. This used to not be a big
+  // deal because we would have to walk up the return path to set
+  // the `childLanes`, anyway, but now those two traversals happen at
+  // different times.
+  // TODO: Consider adding a `root` backpointer on the update queue.
+  detectUpdateOnUnmountedFiber(sourceFiber, sourceFiber);
+  let node = sourceFiber;
+  let parent = node.return;
+  while (parent !== null) {
+    detectUpdateOnUnmountedFiber(sourceFiber, node);
+    node = parent;
+    parent = node.return;
+  }
+  return node.tag === HostRoot ? (node.stateNode: FiberRoot) : null;
+}
+
+function detectUpdateOnUnmountedFiber(sourceFiber: Fiber, parent: Fiber) {
+  if (__DEV__) {
+    const alternate = parent.alternate;
+    if (
+      alternate === null &&
+      (parent.flags & (Placement | Hydrating)) !== NoFlags
+    ) {
+      warnAboutUpdateOnNotYetMountedFiberInDEV(sourceFiber);
+    }
   }
 }