@samitouri / QOS-React / commits / 67b05be0d2

useActionState: Transfer transition context (#29694)

Mini-refactor of useActionState to only wrap the action in a transition context if the dispatch is called during a transition. Conceptually, the action starts as soon as the dispatch is called, even if the action is queued until earlier ones finish. We will also warn if an async action is dispatched outside of a transition, since that is almost certainly a mistake. Ideally we would automatically upgrade these to a transition, but we don't have a great way to tell if the action is async until after it's already run.

Andrew Clark committed Jun 3, 2024 at 11:20 UTC 67b05be0d216c4efebc4bb5acb12c861a18bd87c

2 files changed +282 -139

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

+98 -25

@@ -1020,15 +1020,15 @@ describe('ReactDOMForm', () => {

1020 assertLog(['0']);

1021 expect(container.textContent).toBe('0');

1022

1023 - await act(() => dispatch('increment'));

1023 + await act(() => startTransition(() => dispatch('increment')));

1024 assertLog(['Async action started [1]', 'Pending 0']);

1025 expect(container.textContent).toBe('Pending 0');

1026

1027 // Dispatch a few more actions. None of these will start until the previous

1028 // one finishes.

1029 - await act(() => dispatch('increment'));

1030 - await act(() => dispatch('decrement'));

1031 - await act(() => dispatch('increment'));

1029 + await act(() => startTransition(() => dispatch('increment')));

1030 + await act(() => startTransition(() => dispatch('decrement')));

1031 + await act(() => startTransition(() => dispatch('increment')));

1032 assertLog([]);

1033

1034 // Each action starts as soon as the previous one finishes.

@@ -1067,7 +1067,7 @@ describe('ReactDOMForm', () => {

1067

1068 // Perform an action. This will increase the state by 1, as defined by the

1069 // stepSize prop.

1070 - await act(() => increment());

1070 + await act(() => startTransition(() => increment()));

1071 assertLog(['Pending 0', '1']);

1072

1073 // Now increase the stepSize prop to 10. Subsequent steps will increase

@@ -1076,7 +1076,7 @@ describe('ReactDOMForm', () => {

1076 assertLog(['1']);

1077

1078 // Increment again. The state should increase by 10.

1079 - await act(() => increment());

1079 + await act(() => startTransition(() => increment()));

1080 assertLog(['Pending 1', '11']);

1081 });

1082

@@ -1113,11 +1113,11 @@ describe('ReactDOMForm', () => {

1113 await act(() => root.render(<App />));

1114 assertLog(['A']);

1115

1116 - await act(() => action('B'));

1116 + await act(() => startTransition(() => action('B')));

1117 // The first dispatch will update the pending state.

1118 assertLog(['Pending A']);

1119 - await act(() => action('C'));

1120 - await act(() => action('D'));

1119 + await act(() => startTransition(() => action('C')));

1120 + await act(() => startTransition(() => action('D')));

1121 assertLog([]);

1122

1123 await act(() => resolveText('B'));

@@ -1151,10 +1151,10 @@ describe('ReactDOMForm', () => {

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'));

1154 + await act(() => startTransition(() => action('First action')));

1155 assertLog(['Initial (pending)']);

1156 // This action won't run until the first one finishes.

1157 - await act(() => action('Second action'));

1157 + await act(() => startTransition(() => 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

@@ -1169,7 +1169,9 @@ describe('ReactDOMForm', () => {

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!');

1172 + await expect(

1173 + act(() => startTransition(() => action('Third action'))),

1174 + ).rejects.toThrow('Oops!');

1175 },

1176 );

1177

@@ -1192,7 +1194,7 @@ describe('ReactDOMForm', () => {

1194

1195 // Perform an action. This will increase the state by 1, as defined by the

1196 // stepSize prop.

1195 - await act(() => increment());

1197 + await act(() => startTransition(() => increment()));

1198 assertLog(['Pending 0', '1']);

1199

1200 // Now increase the stepSize prop to 10. Subsequent steps will increase

@@ -1201,7 +1203,7 @@ describe('ReactDOMForm', () => {

1203 assertLog(['1']);

1204

1205 // Increment again. The state should increase by 10.

1204 - await act(() => increment());

1206 + await act(() => startTransition(() => increment()));

1207 assertLog(['Pending 1', '11']);

1208 });

1209

@@ -1219,12 +1221,12 @@ describe('ReactDOMForm', () => {

1221 await act(() => root.render(<App />));

1222 assertLog(['A']);

1223

1222 - await act(() => action(getText('B')));

1224 + await act(() => startTransition(() => action(getText('B'))));

1225 // The first dispatch will update the pending state.

1226 assertLog(['Pending A']);

1225 - await act(() => action('C'));

1226 - await act(() => action(getText('D')));

1227 - await act(() => action('E'));

1227 + await act(() => startTransition(() => action('C')));

1228 + await act(() => startTransition(() => action(getText('D'))));

1229 + await act(() => startTransition(() => action('E')));

1230 assertLog([]);

1231

1232 await act(() => resolveText('B'));

@@ -1273,7 +1275,7 @@ describe('ReactDOMForm', () => {

1275 );

1276 assertLog(['A']);

1277

1276 - await act(() => action('Oops!'));

1278 + await act(() => startTransition(() => action('Oops!')));

1279 assertLog([

1280 // Action begins, error has not thrown yet.

1281 'Pending A',

@@ -1290,8 +1292,8 @@ describe('ReactDOMForm', () => {

1292 // Trigger an error again, but this time, perform another action that

1293 // overrides the first one and fixes the error

1294 await act(() => {

1293 - action('Oops!');

1294 - action('B');

1295 + startTransition(() => action('Oops!'));

1296 + startTransition(() => action('B'));

1297 });

1298 assertLog(['Pending A', 'B']);

1299 expect(container.textContent).toBe('B');

@@ -1338,7 +1340,7 @@ describe('ReactDOMForm', () => {

1340 );

1341 assertLog(['A']);

1342

1341 - await act(() => action('Oops!'));

1343 + await act(() => startTransition(() => action('Oops!')));

1344 // The first dispatch will update the pending state.

1345 assertLog(['Pending A']);

1346 await act(() => resolveText('Oops!'));

@@ -1352,8 +1354,8 @@ describe('ReactDOMForm', () => {

1354 // Trigger an error again, but this time, perform another action that

1355 // overrides the first one and fixes the error

1356 await act(() => {

1355 - action('Oops!');

1356 - action('B');

1357 + startTransition(() => action('Oops!'));

1358 + startTransition(() => action('B'));

1359 });

1360 assertLog(['Pending A']);

1361 await act(() => resolveText('B'));

@@ -1399,7 +1401,7 @@ describe('ReactDOMForm', () => {

1401 assertLog(['0']);

1402 expect(container.textContent).toBe('0');

1403

1402 - await act(() => dispatch('increment'));

1404 + await act(() => startTransition(() => dispatch('increment')));

1405 assertLog(['Async action started [1]', 'Pending 0']);

1406 expect(container.textContent).toBe('Pending 0');

1407

@@ -1408,6 +1410,77 @@ describe('ReactDOMForm', () => {

1410 expect(container.textContent).toBe('1');

1411 });

1412

1413 + test('useActionState does not wrap action in a transition unless dispatch is in a transition', async () => {

1414 + let dispatch;

1415 + function App() {

1416 + const [state, _dispatch] = useActionState(() => {

1417 + return state + 1;

1418 + }, 0);

1419 + dispatch = _dispatch;

1420 + return <AsyncText text={'Count: ' + state} />;

1421 + }

1422 +

1423 + const root = ReactDOMClient.createRoot(container);

1424 + await act(() =>

1425 + root.render(

1426 + <Suspense fallback={<Text text="Loading..." />}>

1427 + <App />

1428 + </Suspense>,

1429 + ),

1430 + );

1431 + assertLog(['Suspend! [Count: 0]', 'Loading...']);

1432 + await act(() => resolveText('Count: 0'));

1433 + assertLog(['Count: 0']);

1434 +

1435 + // Dispatch outside of a transition. This will trigger a loading state.

1436 + await act(() => dispatch());

1437 + assertLog(['Suspend! [Count: 1]', 'Loading...']);

1438 + expect(container.textContent).toBe('Loading...');

1439 +

1440 + await act(() => resolveText('Count: 1'));

1441 + assertLog(['Count: 1']);

1442 + expect(container.textContent).toBe('Count: 1');

1443 +

1444 + // Now dispatch inside of a transition. This one does not trigger a

1445 + // loading state.

1446 + await act(() => startTransition(() => dispatch()));

1447 + assertLog(['Count: 1', 'Suspend! [Count: 2]', 'Loading...']);

1448 + expect(container.textContent).toBe('Count: 1');

1449 +

1450 + await act(() => resolveText('Count: 2'));

1451 + assertLog(['Count: 2']);

1452 + expect(container.textContent).toBe('Count: 2');

1453 + });

1454 +

1455 + test('useActionState warns if async action is dispatched outside of a transition', async () => {

1456 + let dispatch;

1457 + function App() {

1458 + const [state, _dispatch] = useActionState(async () => {

1459 + return state + 1;

1460 + }, 0);

1461 + dispatch = _dispatch;

1462 + return <AsyncText text={'Count: ' + state} />;

1463 + }

1464 +

1465 + const root = ReactDOMClient.createRoot(container);

1466 + await act(() => root.render(<App />));

1467 + assertLog(['Suspend! [Count: 0]']);

1468 + await act(() => resolveText('Count: 0'));

1469 + assertLog(['Count: 0']);

1470 +

1471 + // Dispatch outside of a transition.

1472 + await act(() => dispatch());

1473 + assertConsoleErrorDev([

1474 + [

1475 + 'An async function was passed to useActionState, but it was ' +

1476 + 'dispatched outside of an action context',

1477 + {withoutStack: true},

1478 + ],

1479 + ]);

1480 + assertLog(['Suspend! [Count: 1]']);

1481 + expect(container.textContent).toBe('Count: 0');

1482 + });

1483 +

1484 test('uncontrolled form inputs are reset after the action completes', async () => {

1485 const formRef = React.createRef();

1486 const inputRef = React.createRef();

packages/react-reconciler/src/ReactFiberHooks.js

+184 -114

@@ -2006,65 +2006,87 @@ type ActionStateQueueNode<S, P> = {

2006 action: (Awaited<S>, P) => S,

2007 // This is never null because it's part of a circular linked list.

2008 next: ActionStateQueueNode<S, P>,

2009 +

2010 + // Whether or not the action was dispatched as part of a transition. We use

2011 + // this to restore the transition context when the queued action is run. Once

2012 + // we're able to track parallel async actions, this should be updated to

2013 + // represent the specific transition instance the action is associated with.

2014 + isTransition: boolean,

2015 +

2016 + // Implements the Thenable interface. We use it to suspend until the action

2017 + // finishes.

2018 + then: (listener: () => void) => void,

2019 + status: 'pending' | 'rejected' | 'fulfilled',

2020 + value: any,

2021 + reason: any,

2022 + listeners: Array<() => void>,

2023 };

2024

2025 function dispatchActionState<S, P>(

2026 fiber: Fiber,

2027 actionQueue: ActionStateQueue<S, P>,

2028 setPendingState: boolean => void,

2015 - setState: Dispatch<S | Awaited<S>>,

2029 + setState: Dispatch<ActionStateQueueNode<S, P>>,

2030 payload: P,

2031 ): void {

2032 if (isRenderPhaseUpdate(fiber)) {

2033 throw new Error('Cannot update form state while rendering.');

2034 }

2035 +

2036 + const actionNode: ActionStateQueueNode<S, P> = {

2037 + payload,

2038 + action: actionQueue.action,

2039 + next: (null: any), // circular

2040 +

2041 + isTransition: true,

2042 +

2043 + status: 'pending',

2044 + value: null,

2045 + reason: null,

2046 + listeners: [],

2047 + then(listener) {

2048 + // We know the only thing that subscribes to these promises is `use` so

2049 + // this implementation is simpler than a generic thenable. E.g. we don't

2050 + // bother to check if the thenable is still pending because `use` already

2051 + // does that.

2052 + actionNode.listeners.push(listener);

2053 + },

2054 + };

2055 +

2056 + // Check if we're inside a transition. If so, we'll need to restore the

2057 + // transition context when the action is run.

2058 + const prevTransition = ReactSharedInternals.T;

2059 + if (prevTransition !== null) {

2060 + // Optimistically update the pending state, similar to useTransition.

2061 + // This will be reverted automatically when all actions are finished.

2062 + setPendingState(true);

2063 + // `actionNode` is a thenable that resolves to the return value of

2064 + // the action.

2065 + setState(actionNode);

2066 + } else {

2067 + // This is not a transition.

2068 + actionNode.isTransition = false;

2069 + setState(actionNode);

2070 + }

2071 +

2072 const last = actionQueue.pending;

2073 if (last === null) {

2074 // There are no pending actions; this is the first one. We can run

2075 // it immediately.

2025 - const newLast: ActionStateQueueNode<S, P> = {

2026 - payload,

2027 - action: actionQueue.action,

2028 - next: (null: any), // circular

2029 - };

2030 - newLast.next = actionQueue.pending = newLast;

2031 -

2032 - runActionStateAction(

2033 - actionQueue,

2034 - (setPendingState: any),

2035 - (setState: any),

2036 - newLast,

2037 - );

2076 + actionNode.next = actionQueue.pending = actionNode;

2077 + runActionStateAction(actionQueue, actionNode);

2078 } else {

2079 // There's already an action running. Add to the queue.

2080 const first = last.next;

2041 - const newLast: ActionStateQueueNode<S, P> = {

2042 - payload,

2043 - action: actionQueue.action,

2044 - next: first,

2045 - };

2046 - actionQueue.pending = last.next = newLast;

2081 + actionNode.next = first;

2082 + actionQueue.pending = last.next = actionNode;

2083 }

2084 }

2085

2086 function runActionStateAction<S, P>(

2087 actionQueue: ActionStateQueue<S, P>,

2052 - setPendingState: boolean => void,

2053 - setState: Dispatch<S | Awaited<S>>,

2088 node: ActionStateQueueNode<S, P>,

2089 ) {

2056 - // This is a fork of startTransition

2057 - const prevTransition = ReactSharedInternals.T;

2058 - const currentTransition: BatchConfigTransition = {};

2059 - ReactSharedInternals.T = currentTransition;

2060 - if (__DEV__) {

2061 - ReactSharedInternals.T._updatedFibers = new Set();

2062 - }

2063 -

2064 - // Optimistically update the pending state, similar to useTransition.

2065 - // This will be reverted automatically when all actions are finished.

2066 - setPendingState(true);

2067 -

2090 // `node.action` represents the action function at the time it was dispatched.

2091 // If this action was queued, it might be stale, i.e. it's not necessarily the

2092 // most current implementation of the action, stored on `actionQueue`. This is

@@ -2074,93 +2096,106 @@ function runActionStateAction<S, P>(

2096 const action = node.action;

2097 const payload = node.payload;

2098 const prevState = actionQueue.state;

2077 - try {

2078 - const returnValue = action(prevState, payload);

2079 - const onStartTransitionFinish = ReactSharedInternals.S;

2080 - if (onStartTransitionFinish !== null) {

2081 - onStartTransitionFinish(currentTransition, returnValue);

2099 +

2100 + if (node.isTransition) {

2101 + // The original dispatch was part of a transition. We restore its

2102 + // transition context here.

2103 +

2104 + // This is a fork of startTransition

2105 + const prevTransition = ReactSharedInternals.T;

2106 + const currentTransition: BatchConfigTransition = {};

2107 + ReactSharedInternals.T = currentTransition;

2108 + if (__DEV__) {

2109 + ReactSharedInternals.T._updatedFibers = new Set();

2110 }

2083 - if (

2084 - returnValue !== null &&

2085 - typeof returnValue === 'object' &&

2086 - // $FlowFixMe[method-unbinding]

2087 - typeof returnValue.then === 'function'

2088 - ) {

2089 - const thenable = ((returnValue: any): Thenable<Awaited<S>>);

2090 -

2091 - // Attach a listener to read the return state of the action. As soon as

2092 - // this resolves, we can run the next action in the sequence.

2093 - thenable.then(

2094 - (nextState: Awaited<S>) => {

2095 - actionQueue.state = nextState;

2096 - finishRunningActionStateAction(

2097 - actionQueue,

2098 - (setPendingState: any),

2099 - (setState: any),

2100 - );

2101 - },

2102 - () =>

2103 - finishRunningActionStateAction(

2104 - actionQueue,

2105 - (setPendingState: any),

2106 - (setState: any),

2107 - ),

2108 - );

2111 + try {

2112 + const returnValue = action(prevState, payload);

2113 + const onStartTransitionFinish = ReactSharedInternals.S;

2114 + if (onStartTransitionFinish !== null) {

2115 + onStartTransitionFinish(currentTransition, returnValue);

2116 + }

2117 + handleActionReturnValue(actionQueue, node, returnValue);

2118 + } catch (error) {

2119 + onActionError(actionQueue, node, error);

2120 + } finally {

2121 + ReactSharedInternals.T = prevTransition;

2122

2110 - setState((thenable: any));

2111 - } else {

2112 - setState((returnValue: any));

2113 -

2114 - const nextState = ((returnValue: any): Awaited<S>);

2115 - actionQueue.state = nextState;

2116 - finishRunningActionStateAction(

2117 - actionQueue,

2118 - (setPendingState: any),

2119 - (setState: any),

2120 - );

2123 + if (__DEV__) {

2124 + if (prevTransition === null && currentTransition._updatedFibers) {

2125 + const updatedFibersCount = currentTransition._updatedFibers.size;

2126 + currentTransition._updatedFibers.clear();

2127 + if (updatedFibersCount > 10) {

2128 + console.warn(

2129 + 'Detected a large number of updates inside startTransition. ' +

2130 + 'If this is due to a subscription please re-write it to use React provided hooks. ' +

2131 + 'Otherwise concurrent mode guarantees are off the table.',

2132 + );

2133 + }

2134 + }

2135 + }

2136 }

2122 - } catch (error) {

2123 - // This is a trick to get the `useActionState` hook to rethrow the error.

2124 - // When it unwraps the thenable with the `use` algorithm, the error

2125 - // will be thrown.

2126 - const rejectedThenable: S = ({

2127 - then() {},

2128 - status: 'rejected',

2129 - reason: error,

2130 - // $FlowFixMe: Not sure why this doesn't work

2131 - }: RejectedThenable<Awaited<S>>);

2132 - setState(rejectedThenable);

2133 - finishRunningActionStateAction(

2134 - actionQueue,

2135 - (setPendingState: any),

2136 - (setState: any),

2137 + } else {

2138 + // The original dispatch was not part of a transition.

2139 + try {

2140 + const returnValue = action(prevState, payload);

2141 + handleActionReturnValue(actionQueue, node, returnValue);

2142 + } catch (error) {

2143 + onActionError(actionQueue, node, error);

2144 + }

2145 + }

2146 +}

2147 +

2148 +function handleActionReturnValue<S, P>(

2149 + actionQueue: ActionStateQueue<S, P>,

2150 + node: ActionStateQueueNode<S, P>,

2151 + returnValue: mixed,

2152 +) {

2153 + if (

2154 + returnValue !== null &&

2155 + typeof returnValue === 'object' &&

2156 + // $FlowFixMe[method-unbinding]

2157 + typeof returnValue.then === 'function'

2158 + ) {

2159 + const thenable = ((returnValue: any): Thenable<Awaited<S>>);

2160 + // Attach a listener to read the return state of the action. As soon as

2161 + // this resolves, we can run the next action in the sequence.

2162 + thenable.then(

2163 + (nextState: Awaited<S>) => {

2164 + onActionSuccess(actionQueue, node, nextState);

2165 + },

2166 + (error: mixed) => onActionError(actionQueue, node, error),

2167 );

2138 - } finally {

2139 - ReactSharedInternals.T = prevTransition;

2168

2169 if (__DEV__) {

2142 - if (prevTransition === null && currentTransition._updatedFibers) {

2143 - const updatedFibersCount = currentTransition._updatedFibers.size;

2144 - currentTransition._updatedFibers.clear();

2145 - if (updatedFibersCount > 10) {

2146 - console.warn(

2147 - 'Detected a large number of updates inside startTransition. ' +

2148 - 'If this is due to a subscription please re-write it to use React provided hooks. ' +

2149 - 'Otherwise concurrent mode guarantees are off the table.',

2150 - );

2151 - }

2170 + if (!node.isTransition) {

2171 + console.error(

2172 + 'An async function was passed to useActionState, but it was ' +

2173 + 'dispatched outside of an action context. This is likely not ' +

2174 + 'what you intended. Either pass the dispatch function to an ' +

2175 + '`action` prop, or dispatch manually inside `startTransition`',

2176 + );

2177 }

2178 }

2179 + } else {

2180 + const nextState = ((returnValue: any): Awaited<S>);

2181 + onActionSuccess(actionQueue, node, nextState);

2182 }

2183 }

2184

2157 -function finishRunningActionStateAction<S, P>(

2185 +function onActionSuccess<S, P>(

2186 actionQueue: ActionStateQueue<S, P>,

2159 - setPendingState: Dispatch<S | Awaited<S>>,

2160 - setState: Dispatch<S | Awaited<S>>,

2187 + actionNode: ActionStateQueueNode<S, P>,

2188 + nextState: Awaited<S>,

2189 ) {

2162 - // The action finished running. Pop it from the queue and run the next pending

2163 - // action, if there are any.

2190 + // The action finished running.

2191 + actionNode.status = 'fulfilled';

2192 + actionNode.value = nextState;

2193 + notifyActionListeners(actionNode);

2194 +

2195 + actionQueue.state = nextState;

2196 +

2197 + // Pop the action from the queue and run the next pending action, if there

2198 + // are any.

2199 const last = actionQueue.pending;

2200 if (last !== null) {

2201 const first = last.next;

@@ -2173,16 +2208,51 @@ function finishRunningActionStateAction<S, P>(

2208 last.next = next;

2209

2210 // Run the next action.

2176 - runActionStateAction(

2177 - actionQueue,

2178 - (setPendingState: any),

2179 - (setState: any),

2180 - next,

2181 - );

2211 + runActionStateAction(actionQueue, next);

2212 + }

2213 + }

2214 +}

2215 +

2216 +function onActionError<S, P>(

2217 + actionQueue: ActionStateQueue<S, P>,

2218 + actionNode: ActionStateQueueNode<S, P>,

2219 + error: mixed,

2220 +) {

2221 + actionNode.status = 'rejected';

2222 + actionNode.reason = error;

2223 + notifyActionListeners(actionNode);

2224 +

2225 + // Pop the action from the queue and run the next pending action, if there

2226 + // are any.

2227 + // TODO: We should instead abort all the remaining actions in the queue.

2228 + const last = actionQueue.pending;

2229 + if (last !== null) {

2230 + const first = last.next;

2231 + if (first === last) {

2232 + // This was the last action in the queue.

2233 + actionQueue.pending = null;

2234 + } else {

2235 + // Remove the first node from the circular queue.

2236 + const next = first.next;

2237 + last.next = next;

2238 +

2239 + // Run the next action.

2240 + runActionStateAction(actionQueue, next);

2241 }

2242 }

2243 }

2244

2245 +function notifyActionListeners<S, P>(actionNode: ActionStateQueueNode<S, P>) {

2246 + // Notify React that the action has finished.

2247 + const listeners = actionNode.listeners;

2248 + for (let i = 0; i < listeners.length; i++) {

2249 + // This is always a React internal listener, so we don't need to worry

2250 + // about it throwing.

2251 + const listener = listeners[i];

2252 + listener();

2253 + }

2254 +}

2255 +

2256 function actionStateReducer<S>(oldState: S, newState: S): S {

2257 return newState;

2258 }