@samitouri / QOS-React / commits / 163122766b

Fix: Use action implementation at time of dispatch (#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.

Andrew Clark committed May 28, 2024 at 15:08 UTC 163122766b6008e992898b00f1fe3b104ed78737
2 files changed +67 -12
packages/react-dom/src/__tests__/ReactDOMForm-test.js
+46 -1
@@ -1097,7 +1097,7 @@ describe('ReactDOMForm', () => {
1097 });
1098
1099 // @gate enableAsyncActions
1100 - test('queues multiple actions and runs them in order', async () => {
1100 + test('useActionState: queues multiple actions and runs them in order', async () => {
1101 let action;
1102 function App() {
1103 const [state, dispatch, isPending] = useActionState(
@@ -1128,6 +1128,51 @@ describe('ReactDOMForm', () => {
1128 expect(container.textContent).toBe('D');
1129 });
1130
1131 + // @gate enableAsyncActions
1132 + test(
1133 + 'useActionState: when calling a queued action, uses the implementation ' +
1134 + 'that was current at the time it was dispatched, not the most recent one',
1135 + async () => {
1136 + let action;
1137 + function App({throwIfActionIsDispatched}) {
1138 + const [state, dispatch, isPending] = useActionState(async (s, a) => {
1139 + if (throwIfActionIsDispatched) {
1140 + throw new Error('Oops!');
1141 + }
1142 + return await getText(a);
1143 + }, 'Initial');
1144 + action = dispatch;
1145 + return <Text text={state + (isPending ? ' (pending)' : '')} />;
1146 + }
1147 +
1148 + const root = ReactDOMClient.createRoot(container);
1149 + await act(() => root.render(<App throwIfActionIsDispatched={false} />));
1150 + assertLog(['Initial']);
1151 +
1152 + // Dispatch two actions. The first one is async, so it forces the second
1153 + // one into an async queue.
1154 + await act(() => action('First action'));
1155 + assertLog(['Initial (pending)']);
1156 + // This action won't run until the first one finishes.
1157 + await act(() => action('Second action'));
1158 +
1159 + // While the first action is still pending, update a prop. This causes the
1160 + // inline action implementation to change, but it should not affect the
1161 + // behavior of the action that is already queued.
1162 + await act(() => root.render(<App throwIfActionIsDispatched={true} />));
1163 + assertLog(['Initial (pending)']);
1164 +
1165 + // Finish both of the actions.
1166 + await act(() => resolveText('First action'));
1167 + await act(() => resolveText('Second action'));
1168 + assertLog(['Second action']);
1169 +
1170 + // Confirm that if we dispatch yet another action, it uses the updated
1171 + // action implementation.
1172 + await expect(act(() => action('Third action'))).rejects.toThrow('Oops!');
1173 + },
1174 + );
1175 +
1176 // @gate enableAsyncActions
1177 test('useActionState: works if action is sync', async () => {
1178 let increment;
packages/react-reconciler/src/ReactFiberHooks.js
+21 -11
@@ -1966,13 +1966,15 @@ type ActionStateQueue<S, P> = {
1966 action: (Awaited<S>, P) => S,
1967 // This is a circular linked list of pending action payloads. It incudes the
1968 // action that is currently running.
1969 - pending: ActionStateQueueNode<P> | null,
1969 + pending: ActionStateQueueNode<S, P> | null,
1970 };
1971
1972 -type ActionStateQueueNode<P> = {
1972 +type ActionStateQueueNode<S, P> = {
1973 payload: P,
1974 + // This is the action implementation at the time it was dispatched.
1975 + action: (Awaited<S>, P) => S,
1976 // This is never null because it's part of a circular linked list.
1975 - next: ActionStateQueueNode<P>,
1977 + next: ActionStateQueueNode<S, P>,
1978 };
1979
1980 function dispatchActionState<S, P>(
@@ -1989,8 +1991,9 @@ function dispatchActionState<S, P>(
1991 if (last === null) {
1992 // There are no pending actions; this is the first one. We can run
1993 // it immediately.
1992 - const newLast: ActionStateQueueNode<P> = {
1994 + const newLast: ActionStateQueueNode<S, P> = {
1995 payload,
1996 + action: actionQueue.action,
1997 next: (null: any), // circular
1998 };
1999 newLast.next = actionQueue.pending = newLast;
@@ -1999,13 +2002,14 @@ function dispatchActionState<S, P>(
2002 actionQueue,
2003 (setPendingState: any),
2004 (setState: any),
2002 - payload,
2005 + newLast,
2006 );
2007 } else {
2008 // There's already an action running. Add to the queue.
2009 const first = last.next;
2007 - const newLast: ActionStateQueueNode<P> = {
2010 + const newLast: ActionStateQueueNode<S, P> = {
2011 payload,
2012 + action: actionQueue.action,
2013 next: first,
2014 };
2015 actionQueue.pending = last.next = newLast;
@@ -2016,11 +2020,8 @@ function runActionStateAction<S, P>(
2020 actionQueue: ActionStateQueue<S, P>,
2021 setPendingState: boolean => void,
2022 setState: Dispatch<S | Awaited<S>>,
2019 - payload: P,
2023 + node: ActionStateQueueNode<S, P>,
2024 ) {
2021 - const action = actionQueue.action;
2022 - const prevState = actionQueue.state;
2023 -
2025 // This is a fork of startTransition
2026 const prevTransition = ReactSharedInternals.T;
2027 const currentTransition: BatchConfigTransition = {};
@@ -2033,6 +2034,15 @@ function runActionStateAction<S, P>(
2034 // This will be reverted automatically when all actions are finished.
2035 setPendingState(true);
2036
2037 + // `node.action` represents the action function at the time it was dispatched.
2038 + // If this action was queued, it might be stale, i.e. it's not necessarily the
2039 + // most current implementation of the action, stored on `actionQueue`. This is
2040 + // intentional. The conceptual model for queued actions is that they are
2041 + // queued in a remote worker; the dispatch happens immediately, only the
2042 + // execution is delayed.
2043 + const action = node.action;
2044 + const payload = node.payload;
2045 + const prevState = actionQueue.state;
2046 try {
2047 const returnValue = action(prevState, payload);
2048 const onStartTransitionFinish = ReactSharedInternals.S;
@@ -2136,7 +2146,7 @@ function finishRunningActionStateAction<S, P>(
2146 actionQueue,
2147 (setPendingState: any),
2148 (setState: any),
2139 - next.payload,
2149 + next,
2150 );
2151 }
2152 }