diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index dd3cf8c273d3a..3cdfae52c25db 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1983,8 +1983,6 @@ function runFormStateAction( } try { const returnValue = action(prevState, payload); - notifyTransitionCallbacks(currentTransition, returnValue); - if ( returnValue !== null && typeof returnValue === 'object' && @@ -1992,6 +1990,7 @@ function runFormStateAction( typeof returnValue.then === 'function' ) { const thenable = ((returnValue: any): Thenable>); + notifyTransitionCallbacks(currentTransition, thenable); // Attach a listener to read the return state of the action. As soon as // this resolves, we can run the next action in the sequence. @@ -2854,7 +2853,6 @@ function startTransition( try { if (enableAsyncActions) { const returnValue = callback(); - notifyTransitionCallbacks(currentTransition, returnValue); // Check if we're inside an async action scope. If so, we'll entangle // this new action with the existing scope. @@ -2870,6 +2868,7 @@ function startTransition( typeof returnValue.then === 'function' ) { const thenable = ((returnValue: any): Thenable); + notifyTransitionCallbacks(currentTransition, thenable); // Create a thenable that resolves to `finishedState` once the async // action has completed. const thenableForFinishedState = chainThenableValue( diff --git a/packages/react-reconciler/src/ReactFiberTransition.js b/packages/react-reconciler/src/ReactFiberTransition.js index d420f67dcfe76..6179e9daf5105 100644 --- a/packages/react-reconciler/src/ReactFiberTransition.js +++ b/packages/react-reconciler/src/ReactFiberTransition.js @@ -45,23 +45,17 @@ export function requestCurrentTransition(): BatchConfigTransition | null { if (transition !== null) { // Whenever a transition update is scheduled, register a callback on the // transition object so we can get the return value of the scope function. - transition._callbacks.add(handleTransitionScopeResult); + transition._callbacks.add(handleAsyncAction); } return transition; } -function handleTransitionScopeResult( +function handleAsyncAction( transition: BatchConfigTransition, - returnValue: mixed, + thenable: Thenable, ): void { - if ( - enableAsyncActions && - returnValue !== null && - typeof returnValue === 'object' && - typeof returnValue.then === 'function' - ) { + if (enableAsyncActions) { // This is an async action. - const thenable: Thenable = (returnValue: any); entangleAsyncAction(transition, thenable); } } diff --git a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js index d5003fc49b066..0a29be2b6fec9 100644 --- a/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js +++ b/packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js @@ -12,6 +12,10 @@ describe('ReactAsyncActions', () => { beforeEach(() => { jest.resetModules(); + global.reportError = error => { + Scheduler.log('reportError: ' + error.message); + }; + React = require('react'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); @@ -1726,4 +1730,31 @@ describe('ReactAsyncActions', () => { assertLog(['Async action ended', 'Updated']); expect(root).toMatchRenderedOutput(Updated); }); + + test('React.startTransition captures async errors and passes them to reportError', async () => { + // NOTE: This is gated here instead of using the pragma because the failure + // happens asynchronously and the `gate` runtime doesn't capture it. + if (gate(flags => flags.enableAsyncActions)) { + await act(() => { + React.startTransition(async () => { + throw new Error('Oops'); + }); + }); + assertLog(['reportError: Oops']); + } + }); + + // @gate enableAsyncActions + test('React.startTransition captures sync errors and passes them to reportError', async () => { + await act(() => { + try { + React.startTransition(() => { + throw new Error('Oops'); + }); + } catch (e) { + throw new Error('Should not be reachable.'); + } + }); + assertLog(['reportError: Oops']); + }); }); diff --git a/packages/react/src/ReactStartTransition.js b/packages/react/src/ReactStartTransition.js index ab956a2d2cb38..ebdc0d7718a66 100644 --- a/packages/react/src/ReactStartTransition.js +++ b/packages/react/src/ReactStartTransition.js @@ -10,7 +10,10 @@ import type {BatchConfigTransition} from 'react-reconciler/src/ReactFiberTracing import type {StartTransitionOptions} from 'shared/ReactTypes'; import ReactCurrentBatchConfig from './ReactCurrentBatchConfig'; -import {enableTransitionTracing} from 'shared/ReactFeatureFlags'; +import { + enableAsyncActions, + enableTransitionTracing, +} from 'shared/ReactFeatureFlags'; export function startTransition( scope: () => void, @@ -39,24 +42,65 @@ export function startTransition( } } - try { - const returnValue = scope(); - callbacks.forEach(callback => callback(currentTransition, returnValue)); - } finally { - ReactCurrentBatchConfig.transition = prevTransition; - - if (__DEV__) { - if (prevTransition === null && currentTransition._updatedFibers) { - const updatedFibersCount = currentTransition._updatedFibers.size; - currentTransition._updatedFibers.clear(); - if (updatedFibersCount > 10) { - console.warn( - 'Detected a large number of updates inside startTransition. ' + - 'If this is due to a subscription please re-write it to use React provided hooks. ' + - 'Otherwise concurrent mode guarantees are off the table.', - ); - } + if (enableAsyncActions) { + try { + const returnValue = scope(); + if ( + typeof returnValue === 'object' && + returnValue !== null && + typeof returnValue.then === 'function' + ) { + callbacks.forEach(callback => callback(currentTransition, returnValue)); + returnValue.then(noop, onError); + } + } catch (error) { + onError(error); + } finally { + warnAboutTransitionSubscriptions(prevTransition, currentTransition); + ReactCurrentBatchConfig.transition = prevTransition; + } + } else { + // When async actions are not enabled, startTransition does not + // capture errors. + try { + scope(); + } finally { + warnAboutTransitionSubscriptions(prevTransition, currentTransition); + ReactCurrentBatchConfig.transition = prevTransition; + } + } +} + +function warnAboutTransitionSubscriptions( + prevTransition: BatchConfigTransition | null, + currentTransition: BatchConfigTransition, +) { + if (__DEV__) { + if (prevTransition === null && currentTransition._updatedFibers) { + const updatedFibersCount = currentTransition._updatedFibers.size; + currentTransition._updatedFibers.clear(); + if (updatedFibersCount > 10) { + console.warn( + 'Detected a large number of updates inside startTransition. ' + + 'If this is due to a subscription please re-write it to use React provided hooks. ' + + 'Otherwise concurrent mode guarantees are off the table.', + ); } } } } + +function noop() {} + +// Use reportError, if it exists. Otherwise console.error. This is the same as +// the default for onRecoverableError. +const onError = + typeof reportError === 'function' + ? // In modern browsers, reportError will dispatch an error event, + // emulating an uncaught JavaScript error. + reportError + : (error: mixed) => { + // In older browsers and test environments, fallback to console.error. + // eslint-disable-next-line react-internal/no-production-logging + console['error'](error); + };