Bugfix: "Captured" updates on legacy queue (#18265)

* Bugfix: "Captured" updates on legacy queue

This fixes a bug with error boundaries. Error boundaries have a notion
of "captured" updates that represent errors that are thrown in its
subtree during the render phase. These updates are meant to be dropped
if the render is aborted.

The bug happens when there's a concurrent update (an update from an
interleaved event) in between when the error is thrown and when the
error boundary does its second pass. The concurrent update is
transferred from the pending queue onto the base queue. Usually, at this
point the base queue is the same as the current queue. So when we
append the pending updates to the work-in-progress queue, it also
appends to the current queue.

However, in the case of an error boundary's second pass, the base queue
has already forked from the current queue; it includes both the
"captured" updates and any concurrent updates. In that case, what we
need to do is append separately to both queues. Which we weren't doing.

That isn't the full story, though. You would expect that this mistake
would manifest as dropping the interleaved updates. But instead what
was happening is that the "captured" updates, the ones that are meant
to be dropped if the render is aborted, were being added to the
current queue.

The reason is that the `baseQueue` structure is a circular linked list.
The motivation for this was to save memory; instead of separate `first`
and `last` pointers, you only need to point to `last`.

But this approach does not work with structural sharing. So what was
happening is that the captured updates were accidentally being added
to the current queue because of the circular link.

To fix this, I changed the `baseQueue` from a circular linked list to a
singly-linked list so that we can take advantage of structural sharing.

The "pending" queue, however, remains a circular list because it doesn't
need to be persistent.

This bug also affects the root fiber, which uses the same update queue
implementation and also acts like an error boundary.

It does not affect the hook update queue because they do not have any
notion of "captured" updates. So I've left it alone for now. However,
when we implement resuming, we will have to account for the same issue.

* Ensure base queue is a clone

When an error boundary captures an error, we append the error update
to the work-in-progress queue only so that if the render is aborted,
the error update is dropped.

Before appending to the queue, we need to make sure the queue is a
work-in-progress copy. Usually we clone the queue during
`processUpdateQueue`; however, if the base queue has lower priority
than the current render, we may have bailed out on the boundary fiber
without ever entering `processUpdateQueue`. So we need to lazily clone
the queue.

* Add warning to protect against refactor hazard

The hook queue does not have resuming or "captured" updates, but if
we ever add them in the future, we'll need to make sure we check if the
queue is forked before transfering the pending updates to them.

Author: acdlite

packages/react-noop-renderer/src/createReactNoop.js

@@ -984,19 +984,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 
       function logUpdateQueue(updateQueue: UpdateQueue<mixed>, depth) {
         log('  '.repeat(depth + 1) + 'QUEUED UPDATES');
-        const last = updateQueue.baseQueue;
-        if (last === null) {
-          return;
-        }
-        const first = last.next;
+        const first = updateQueue.firstBaseUpdate;
         let update = first;
         if (update !== null) {
           do {
             log(
               '  '.repeat(depth + 1) + '~',
               '[' + update.expirationTime + ']',
             );
-          } while (update !== null && update !== first);
+          } while (update !== null);
         }
 
         const lastPending = updateQueue.shared.pending;

packages/react-reconciler/src/ReactFiberHooks.js

@@ -691,6 +691,16 @@ function updateReducer<S, I, A>(
       baseQueue.next = pendingFirst;
       pendingQueue.next = baseFirst;
     }
+    if (__DEV__) {
+      if (current.baseQueue !== baseQueue) {
+        // Internal invariant that should never happen, but feasibly could in
+        // the future if we implement resuming, or some form of that.
+        console.error(
+          'Internal error: Expected work-in-progress queue to be a clone. ' +
+            'This is a bug in React.',
+        );
+      }
+    }
     current.baseQueue = baseQueue = pendingQueue;
     queue.pending = null;
   }