diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 24746bde8d330..ae4b68b592d5c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -143,6 +143,7 @@ import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent'; import { requestAsyncActionContext, requestSyncActionContext, + peekEntangledActionLane, } from './ReactFiberAsyncAction'; import {HostTransitionContext} from './ReactFiberHostContext'; import {requestTransitionLane} from './ReactFiberRootScheduler'; @@ -2722,6 +2723,7 @@ function startTransition( ); const prevTransition = ReactCurrentBatchConfig.transition; + const currentTransition: BatchConfigTransition = {}; if (enableAsyncActions) { // We don't really need to use an optimistic update here, because we @@ -2730,15 +2732,14 @@ function startTransition( // optimistic update anyway to make it less likely the behavior accidentally // diverges; for example, both an optimistic update and this one should // share the same lane. + ReactCurrentBatchConfig.transition = currentTransition; dispatchOptimisticSetState(fiber, false, queue, pendingState); } else { ReactCurrentBatchConfig.transition = null; dispatchSetState(fiber, queue, pendingState); + ReactCurrentBatchConfig.transition = currentTransition; } - const currentTransition = (ReactCurrentBatchConfig.transition = - ({}: BatchConfigTransition)); - if (enableTransitionTracing) { if (options !== undefined && options.name !== undefined) { ReactCurrentBatchConfig.transition.name = options.name; @@ -3201,14 +3202,48 @@ function dispatchOptimisticSetState( queue: UpdateQueue, action: A, ): void { + if (__DEV__) { + if (ReactCurrentBatchConfig.transition === null) { + // An optimistic update occurred, but startTransition is not on the stack. + // There are two likely scenarios. + + // One possibility is that the optimistic update is triggered by a regular + // event handler (e.g. `onSubmit`) instead of an action. This is a mistake + // and we will warn. + + // The other possibility is the optimistic update is inside an async + // action, but after an `await`. In this case, we can make it "just work" + // by associating the optimistic update with the pending async action. + + // Technically it's possible that the optimistic update is unrelated to + // the pending action, but we don't have a way of knowing this for sure + // because browsers currently do not provide a way to track async scope. + // (The AsyncContext proposal, if it lands, will solve this in the + // future.) However, this is no different than the problem of unrelated + // transitions being grouped together — it's not wrong per se, but it's + // not ideal. + + // Once AsyncContext starts landing in browsers, we will provide better + // warnings in development for these cases. + if (peekEntangledActionLane() !== NoLane) { + // There is a pending async action. Don't warn. + } else { + // There's no pending async action. The most likely cause is that we're + // inside a regular event handler (e.g. onSubmit) instead of an action. + console.error( + 'An optimistic state update occurred outside a transition or ' + + 'action. To fix, move the update to an action, or wrap ' + + 'with startTransition.', + ); + } + } + } + const update: Update = { // An optimistic update commits synchronously. lane: SyncLane, // After committing, the optimistic update is "reverted" using the same // lane as the transition it's associated with. - // - // TODO: Warn if there's no transition/action associated with this - // optimistic update. revertLane: requestTransitionLane(), action, hasEagerState: false, diff --git a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js index 13143fa5ecf5b..2a36c4cb8c832 100644 --- a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js +++ b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js @@ -1142,7 +1142,7 @@ describe('ReactAsyncActions', () => { // Initial render const root = ReactNoop.createRoot(); - await act(() => root.render()); + await act(() => root.render()); assertLog(['A']); expect(root).toMatchRenderedOutput(
A
); @@ -1174,5 +1174,54 @@ describe('ReactAsyncActions', () => { await act(() => resolveText('Wait 2')); assertLog(['B']); + expect(root).toMatchRenderedOutput(
B
); + }); + + // @gate enableAsyncActions + test('useOptimistic warns if outside of a transition', async () => { + let startTransition; + let setLoadingProgress; + let setText; + function App() { + const [, _startTransition] = useTransition(); + const [text, _setText] = useState('A'); + const [loadingProgress, _setLoadingProgress] = useOptimistic(0); + startTransition = _startTransition; + setText = _setText; + setLoadingProgress = _setLoadingProgress; + + return ( + <> + {loadingProgress !== 0 ? ( +
+ +
+ ) : null} +
+ +
+ + ); + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(() => root.render()); + assertLog(['A']); + expect(root).toMatchRenderedOutput(
A
); + + await expect(async () => { + await act(() => { + setLoadingProgress('25%'); + startTransition(() => setText('B')); + }); + }).toErrorDev( + 'An optimistic state update occurred outside a transition or ' + + 'action. To fix, move the update to an action, or wrap ' + + 'with startTransition.', + {withoutStack: true}, + ); + assertLog(['Loading... (25%)', 'A', 'B']); + expect(root).toMatchRenderedOutput(
B
); }); });