Fix: Use action implementation at time of dispatch (facebook#29618)

Fixes the behavior of actions that are queued by useActionState to use
the action function that was current at the time it was dispatched, not
at the time it eventually executes.

The conceptual model is that the action is immediately dispatched, as if
it were sent to a remote server/worker. It's the remote worker that
maintains the queue, not the client.

This is another property of actions makes them more like event handlers
than like reducers.

Author: acdlite

packages/react-dom/src/__tests__/ReactDOMForm-test.js

@@ -1097,7 +1097,7 @@ describe('ReactDOMForm', () => {
   });
 
   // @gate enableAsyncActions
-  test('queues multiple actions and runs them in order', async () => {
+  test('useActionState: queues multiple actions and runs them in order', async () => {
     let action;
     function App() {
       const [state, dispatch, isPending] = useActionState(
@@ -1128,6 +1128,51 @@ describe('ReactDOMForm', () => {
     expect(container.textContent).toBe('D');
   });
 
+  // @gate enableAsyncActions
+  test(
+    'useActionState: when calling a queued action, uses the implementation ' +
+      'that was current at the time it was dispatched, not the most recent one',
+    async () => {
+      let action;
+      function App({throwIfActionIsDispatched}) {
+        const [state, dispatch, isPending] = useActionState(async (s, a) => {
+          if (throwIfActionIsDispatched) {
+            throw new Error('Oops!');
+          }
+          return await getText(a);
+        }, 'Initial');
+        action = dispatch;
+        return <Text text={state + (isPending ? ' (pending)' : '')} />;
+      }
+
+      const root = ReactDOMClient.createRoot(container);
+      await act(() => root.render(<App throwIfActionIsDispatched={false} />));
+      assertLog(['Initial']);
+
+      // Dispatch two actions. The first one is async, so it forces the second
+      // one into an async queue.
+      await act(() => action('First action'));
+      assertLog(['Initial (pending)']);
+      // This action won't run until the first one finishes.
+      await act(() => action('Second action'));
+
+      // While the first action is still pending, update a prop. This causes the
+      // inline action implementation to change, but it should not affect the
+      // behavior of the action that is already queued.
+      await act(() => root.render(<App throwIfActionIsDispatched={true} />));
+      assertLog(['Initial (pending)']);
+
+      // Finish both of the actions.
+      await act(() => resolveText('First action'));
+      await act(() => resolveText('Second action'));
+      assertLog(['Second action']);
+
+      // Confirm that if we dispatch yet another action, it uses the updated
+      // action implementation.
+      await expect(act(() => action('Third action'))).rejects.toThrow('Oops!');
+    },
+  );
+
   // @gate enableAsyncActions
   test('useActionState: works if action is sync', async () => {
     let increment;

packages/react-reconciler/src/ReactFiberHooks.js

@@ -1966,13 +1966,15 @@ type ActionStateQueue<S, P> = {
   action: (Awaited<S>, P) => S,
   // This is a circular linked list of pending action payloads. It incudes the
   // action that is currently running.
-  pending: ActionStateQueueNode<P> | null,
+  pending: ActionStateQueueNode<S, P> | null,
 };
 
-type ActionStateQueueNode<P> = {
+type ActionStateQueueNode<S, P> = {
   payload: P,
+  // This is the action implementation at the time it was dispatched.
+  action: (Awaited<S>, P) => S,
   // This is never null because it's part of a circular linked list.
-  next: ActionStateQueueNode<P>,
+  next: ActionStateQueueNode<S, P>,
 };
 
 function dispatchActionState<S, P>(
@@ -1989,8 +1991,9 @@ function dispatchActionState<S, P>(
   if (last === null) {
     // There are no pending actions; this is the first one. We can run
     // it immediately.
-    const newLast: ActionStateQueueNode<P> = {
+    const newLast: ActionStateQueueNode<S, P> = {
       payload,
+      action: actionQueue.action,
       next: (null: any), // circular
     };
     newLast.next = actionQueue.pending = newLast;
@@ -1999,13 +2002,14 @@ function dispatchActionState<S, P>(
       actionQueue,
       (setPendingState: any),
       (setState: any),
-      payload,
+      newLast,
     );
   } else {
     // There's already an action running. Add to the queue.
     const first = last.next;
-    const newLast: ActionStateQueueNode<P> = {
+    const newLast: ActionStateQueueNode<S, P> = {
       payload,
+      action: actionQueue.action,
       next: first,
     };
     actionQueue.pending = last.next = newLast;
@@ -2016,11 +2020,8 @@ function runActionStateAction<S, P>(
   actionQueue: ActionStateQueue<S, P>,
   setPendingState: boolean => void,
   setState: Dispatch<S | Awaited<S>>,
-  payload: P,
+  node: ActionStateQueueNode<S, P>,
 ) {
-  const action = actionQueue.action;
-  const prevState = actionQueue.state;
-
   // This is a fork of startTransition
   const prevTransition = ReactSharedInternals.T;
   const currentTransition: BatchConfigTransition = {};
@@ -2033,6 +2034,15 @@ function runActionStateAction<S, P>(
   // This will be reverted automatically when all actions are finished.
   setPendingState(true);
 
+  // `node.action` represents the action function at the time it was dispatched.
+  // If this action was queued, it might be stale, i.e. it's not necessarily the
+  // most current implementation of the action, stored on `actionQueue`. This is
+  // intentional. The conceptual model for queued actions is that they are
+  // queued in a remote worker; the dispatch happens immediately, only the
+  // execution is delayed.
+  const action = node.action;
+  const payload = node.payload;
+  const prevState = actionQueue.state;
   try {
     const returnValue = action(prevState, payload);
     const onStartTransitionFinish = ReactSharedInternals.S;
@@ -2136,7 +2146,7 @@ function finishRunningActionStateAction<S, P>(
         actionQueue,
         (setPendingState: any),
         (setState: any),
-        next.payload,
+        next,
       );
     }
   }