diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js index 04cfc5525e6..0d9aad9ec04 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js @@ -625,13 +625,11 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - // Fiber mounts with null children before capturing error - 'ErrorBoundary componentDidMount', // Catch and render an error message 'ErrorBoundary componentDidCatch', - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -653,13 +651,11 @@ describe('ReactErrorBoundaries', () => { 'ErrorBoundary componentWillMount', 'ErrorBoundary render success', 'BrokenConstructor constructor [!]', - // Fiber mounts with null children before capturing error - 'ErrorBoundary componentDidMount', // Catch and render an error message 'ErrorBoundary componentDidCatch', - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -682,11 +678,10 @@ describe('ReactErrorBoundaries', () => { 'ErrorBoundary render success', 'BrokenComponentWillMount constructor', 'BrokenComponentWillMount componentWillMount [!]', - 'ErrorBoundary componentDidMount', 'ErrorBoundary componentDidCatch', - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -765,15 +760,14 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - 'ErrorBoundary componentDidMount', 'ErrorBoundary componentDidCatch', - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', 'ErrorMessage constructor', 'ErrorMessage componentWillMount', 'ErrorMessage render', 'ErrorMessage componentDidMount', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -805,22 +799,19 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - // In Fiber, failed error boundaries render null before attempting to recover - 'RetryErrorBoundary componentDidMount', 'RetryErrorBoundary componentDidCatch [!]', - 'ErrorBoundary componentDidMount', + 'RetryErrorBoundary componentWillMount', // Retry 'RetryErrorBoundary render', 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', // This time, the error propagates to the higher boundary - 'RetryErrorBoundary componentWillUnmount', 'ErrorBoundary componentDidCatch', // Render the error - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -828,7 +819,7 @@ describe('ReactErrorBoundaries', () => { expect(log).toEqual(['ErrorBoundary componentWillUnmount']); }); - it('propagates errors inside boundary during componentWillMount', () => { + fit('propagates errors inside boundary during componentWillMount', () => { const container = document.createElement('div'); ReactDOM.render( @@ -844,11 +835,10 @@ describe('ReactErrorBoundaries', () => { 'BrokenComponentWillMountErrorBoundary constructor', 'BrokenComponentWillMountErrorBoundary componentWillMount [!]', // The error propagates to the higher boundary - 'ErrorBoundary componentDidMount', 'ErrorBoundary componentDidCatch', - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -879,19 +869,16 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender render [!]', // The first error boundary catches the error // It adjusts state but throws displaying the message - // Finish mounting with null children - 'BrokenRenderErrorBoundary componentDidMount', // Attempt to handle the error 'BrokenRenderErrorBoundary componentDidCatch', - 'ErrorBoundary componentDidMount', + 'BrokenRenderErrorBoundary componentWillMount', 'BrokenRenderErrorBoundary render error [!]', // Boundary fails with new error, propagate to next boundary - 'BrokenRenderErrorBoundary componentWillUnmount', // Attempt to handle the error again 'ErrorBoundary componentDidCatch', - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -922,14 +909,16 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - // Finish mounting with null children - 'ErrorBoundary componentDidMount', - // Handle the error + // Continue rendering siblings + 'Normal constructor', + 'Normal componentWillMount', + 'Normal render', + // Capture the error 'ErrorBoundary componentDidCatch', // Render the error message - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -961,16 +950,13 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - // Handle error: - // Finish mounting with null children - 'ErrorBoundary componentDidMount', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', // Render the error message - 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary componentWillMount', 'ErrorBoundary render error', 'Error message ref is set to [object HTMLDivElement]', - 'ErrorBoundary componentDidUpdate', + 'ErrorBoundary componentDidMount', ]); log.length = 0; @@ -1034,14 +1020,12 @@ describe('ReactErrorBoundaries', () => { 'Normal2 render', // BrokenConstructor will abort rendering: 'BrokenConstructor constructor [!]', - // Finish updating with null children - 'Normal componentWillUnmount', - 'ErrorBoundary componentDidUpdate', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', // Render the error message 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'Normal componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1083,14 +1067,12 @@ describe('ReactErrorBoundaries', () => { // BrokenComponentWillMount will abort rendering: 'BrokenComponentWillMount constructor', 'BrokenComponentWillMount componentWillMount [!]', - // Finish updating with null children - 'Normal componentWillUnmount', - 'ErrorBoundary componentDidUpdate', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', // Render the error message 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'Normal componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1127,14 +1109,12 @@ describe('ReactErrorBoundaries', () => { 'Normal render', // BrokenComponentWillReceiveProps will abort rendering: 'BrokenComponentWillReceiveProps componentWillReceiveProps [!]', - // Finish updating with null children - 'Normal componentWillUnmount', - 'BrokenComponentWillReceiveProps componentWillUnmount', - 'ErrorBoundary componentDidUpdate', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'Normal componentWillUnmount', + 'BrokenComponentWillReceiveProps componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1172,14 +1152,12 @@ describe('ReactErrorBoundaries', () => { // BrokenComponentWillUpdate will abort rendering: 'BrokenComponentWillUpdate componentWillReceiveProps', 'BrokenComponentWillUpdate componentWillUpdate [!]', - // Finish updating with null children - 'Normal componentWillUnmount', - 'BrokenComponentWillUpdate componentWillUnmount', - 'ErrorBoundary componentDidUpdate', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'Normal componentWillUnmount', + 'BrokenComponentWillUpdate componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1222,13 +1200,11 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - // Finish updating with null children - 'Normal componentWillUnmount', - 'ErrorBoundary componentDidUpdate', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'Normal componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1281,15 +1257,13 @@ describe('ReactErrorBoundaries', () => { 'BrokenRender constructor', 'BrokenRender componentWillMount', 'BrokenRender render [!]', - // Finish updating with null children - 'Child1 ref is set to null', - 'ErrorBoundary componentDidUpdate', - // Handle the error + // Capture the error 'ErrorBoundary componentDidCatch', 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', - 'Error message ref is set to [object HTMLDivElement]', + 'Child1 ref is set to null', // Child2 ref is never set because its mounting aborted + 'Error message ref is set to [object HTMLDivElement]', 'ErrorBoundary componentDidUpdate', ]); @@ -1307,7 +1281,7 @@ describe('ReactErrorBoundaries', () => { - + Normal , container, ); @@ -1328,15 +1302,25 @@ describe('ReactErrorBoundaries', () => { 'BrokenComponentWillUnmount componentWillReceiveProps', 'BrokenComponentWillUnmount componentWillUpdate', 'BrokenComponentWillUnmount render', - // Unmounting throws: + // The first error boundary is unmounted, which throws: 'BrokenComponentWillUnmount componentWillUnmount [!]', // Fiber proceeds with lifecycles despite errors 'Normal componentWillUnmount', // The components have updated in this phase 'BrokenComponentWillUnmount componentDidUpdate', 'ErrorBoundary componentDidUpdate', - // Now that commit phase is done, Fiber unmounts the boundary's children + // Now that commit phase is done, we update the error boundaries + // Capture the error + 'ErrorBoundary componentDidCatch', + // The error handler schedules an update + 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary render error', + + // The second, sibling error boundary will be unmounted. This + // triggers *another* error. 'BrokenComponentWillUnmount componentWillUnmount [!]', + 'ErrorBoundary componentDidUpdate', + // Capture the error 'ErrorBoundary componentDidCatch', // The initial render was aborted, so // Fiber retries from the root. @@ -1386,20 +1370,23 @@ describe('ReactErrorBoundaries', () => { 'BrokenComponentWillUnmount componentWillReceiveProps', 'BrokenComponentWillUnmount componentWillUpdate', 'BrokenComponentWillUnmount render', - // Unmounting throws: + // Unmount the innermost BrokenComponentWillUnmount, which throws. 'BrokenComponentWillUnmount componentWillUnmount [!]', // Fiber proceeds with lifecycles despite errors 'BrokenComponentWillUnmount componentDidUpdate', 'Normal componentDidUpdate', 'ErrorBoundary componentDidUpdate', + // Now that commit phase is done, capture the error: + 'ErrorBoundary componentDidCatch', + 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary render error', 'Normal componentWillUnmount', + // Now the second error boundary is unmounted. 'BrokenComponentWillUnmount componentWillUnmount [!]', - // Now that commit phase is done, Fiber handles errors + 'ErrorBoundary componentDidUpdate', + // Capture the second error 'ErrorBoundary componentDidCatch', - // The initial render was aborted, so - // Fiber retries from the root. 'ErrorBoundary componentWillUpdate', - // Render an error now (stack will do it later) 'ErrorBoundary render error', // Done 'ErrorBoundary componentDidUpdate', @@ -1714,13 +1701,19 @@ describe('ReactErrorBoundaries', () => { 'LastChild componentDidMount', 'ErrorBoundary componentDidMount', // Now we are ready to handle the error - // Safely unmount every child + // Capture the error + 'ErrorBoundary componentDidCatch', + 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary render error', + // The second BrokenComponentWillUnmount is unmounted, triggering + // another error. 'BrokenComponentWillUnmount componentWillUnmount [!]', // Continue unmounting safely despite any errors 'Normal componentWillUnmount', 'BrokenComponentDidMount componentWillUnmount', 'LastChild componentWillUnmount', - // Handle the error + 'ErrorBoundary componentDidUpdate', + // Capture the second error 'ErrorBoundary componentDidCatch', 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', @@ -1759,11 +1752,11 @@ describe('ReactErrorBoundaries', () => { // All lifecycles run 'BrokenComponentDidUpdate componentDidUpdate [!]', 'ErrorBoundary componentDidUpdate', - 'BrokenComponentDidUpdate componentWillUnmount', // Then, error is handled 'ErrorBoundary componentDidCatch', 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'BrokenComponentDidUpdate componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1795,12 +1788,12 @@ describe('ReactErrorBoundaries', () => { 'BrokenComponentDidMountErrorBoundary componentDidMount [!]', // Fiber proceeds with the hooks 'ErrorBoundary componentDidMount', - 'BrokenComponentDidMountErrorBoundary componentWillUnmount', // The error propagates to the higher boundary 'ErrorBoundary componentDidCatch', // Fiber retries from the root 'ErrorBoundary componentWillUpdate', 'ErrorBoundary render error', + 'BrokenComponentDidMountErrorBoundary componentWillUnmount', 'ErrorBoundary componentDidUpdate', ]); @@ -1809,7 +1802,7 @@ describe('ReactErrorBoundaries', () => { expect(log).toEqual(['ErrorBoundary componentWillUnmount']); }); - it('lets different boundaries catch their own first errors', () => { + it('lets different boundaries catch their own errors', () => { function renderUnmountError(error) { return
Caught an unmounting error: {error.message}.
; } @@ -1854,7 +1847,7 @@ describe('ReactErrorBoundaries', () => { ); expect(container.firstChild.textContent).toBe( - 'Caught an unmounting error: E1.' + 'Caught an updating error: E3.', + 'Caught an unmounting error: E2.' + 'Caught an updating error: E4.', ); expect(log).toEqual([ // Begin update phase @@ -1887,14 +1880,16 @@ describe('ReactErrorBoundaries', () => { 'OuterErrorBoundary componentDidUpdate', // After the commit phase, attempt to recover from any errors that // were captured - 'BrokenComponentDidUpdate componentWillUnmount', - 'BrokenComponentDidUpdate componentWillUnmount', 'InnerUnmountBoundary componentDidCatch', - 'InnerUpdateBoundary componentDidCatch', + 'InnerUnmountBoundary componentDidCatch', 'InnerUnmountBoundary componentWillUpdate', 'InnerUnmountBoundary render error', + 'InnerUpdateBoundary componentDidCatch', + 'InnerUpdateBoundary componentDidCatch', 'InnerUpdateBoundary componentWillUpdate', 'InnerUpdateBoundary render error', + 'BrokenComponentDidUpdate componentWillUnmount', + 'BrokenComponentDidUpdate componentWillUnmount', 'InnerUnmountBoundary componentDidUpdate', 'InnerUpdateBoundary componentDidUpdate', ]); @@ -1933,34 +1928,41 @@ describe('ReactErrorBoundaries', () => { expect(err2.message).toMatch(/got: undefined/); }); - it('renders empty output if error boundary does not handle the error', () => { + it('propagates to next boundary if boundary does not handle the error', () => { const container = document.createElement('div'); - ReactDOM.render( -
- Sibling - - - -
, - container, - ); - expect(container.firstChild.textContent).toBe('Sibling'); + expect(() => + ReactDOM.render( +
+ Sibling + + + +
, + container, + ), + ).toThrow('Hello'); + expect(container.firstChild).toBe(null); expect(log).toEqual([ 'NoopErrorBoundary constructor', 'NoopErrorBoundary componentWillMount', 'NoopErrorBoundary render', 'BrokenRender constructor', 'BrokenRender componentWillMount', + // Render throws an error 'BrokenRender render [!]', - // In Fiber, noop error boundaries render null - 'NoopErrorBoundary componentDidMount', + // Capture the error and retry 'NoopErrorBoundary componentDidCatch', - // Nothing happens. + 'NoopErrorBoundary render', + // The same failure happens again + 'BrokenRender constructor', + 'BrokenRender componentWillMount', + 'BrokenRender render [!]', + // This time, the error propagates up to the root and is thrown ]); log.length = 0; ReactDOM.unmountComponentAtNode(container); - expect(log).toEqual(['NoopErrorBoundary componentWillUnmount']); + expect(log).toEqual([]); }); it('passes first error when two errors happen in commit', () => { diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index b0335506418..e5fe63b418b 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -1362,32 +1362,27 @@ describe('ReactUpdates', () => { }); it('does not fall into an infinite error loop', () => { - function BadRender() { - throw new Error('error'); - } - - class ErrorBoundary extends React.Component { - componentDidCatch() { - this.props.parent.remount(); + class BadMount extends React.Component { + componentDidMount() { + throw new Error('error'); } render() { - return ; + return null; } } - class NonTerminating extends React.Component { - state = {step: 0}; - remount() { - this.setState(state => ({step: state.step + 1})); + class ErrorBoundary extends React.Component { + componentDidCatch() { + // Noop } render() { - return ; + return ; } } const container = document.createElement('div'); expect(() => { - ReactDOM.render(, container); + ReactDOM.render(, container); }).toThrow('Maximum'); }); }); diff --git a/packages/react-reconciler/src/ReactCapturedValue.js b/packages/react-reconciler/src/ReactCapturedValue.js new file mode 100644 index 00000000000..c8083838b1d --- /dev/null +++ b/packages/react-reconciler/src/ReactCapturedValue.js @@ -0,0 +1,134 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {Fiber} from './ReactFiber'; + +import {ClassComponent} from 'shared/ReactTypeOfWork'; +import getComponentName from 'shared/getComponentName'; +import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook'; + +import {logCapturedError} from './ReactFiberErrorLogger'; + +const getPrototypeOf = + Object.getPrototypeOf === 'function' ? Object.getPrototypeOf : null; +const objectToString = Object.prototype.toString; + +export type CapturedValue = { + value: T, + isError: boolean, + source: Fiber | null, + boundary: Fiber | null, + stack: string | null, +}; + +// Object that is passed to the error logger module. +// TODO: CapturedError is different from CapturedValue for legacy reasons, but I +// don't think it's exposed to anyone outside FB, so we can probably change it. +export type CapturedError = { + componentName: string | null, + componentStack: string, + error: mixed, + errorBoundary: Fiber | null, + errorBoundaryFound: boolean, + errorBoundaryName: string | null, + willRetry: boolean, +}; + +// Call this immediately after the value is thrown. +export function createCapturedValue( + value: T, + source: Fiber | null, +): CapturedValue { + const valueIsError = isError(value); + return { + value, + isError: valueIsError, + source, + boundary: null, + // Don't compute the stack unless this is an error. + stack: + source !== null && valueIsError + ? getStackAddendumByWorkInProgressFiber(source) + : null, + }; +} + +export function logError(capturedValue: CapturedValue): void { + const capturedError = createCapturedError(capturedValue); + try { + logCapturedError(capturedError); + } catch (e) { + // Prevent cycle if logCapturedError() throws. + // A cycle may still occur if logCapturedError renders a component that throws. + const suppressLogging = e && e.suppressReactErrorLogging; + if (!suppressLogging) { + console.error(e); + } + } +} + +// Create a CapturedError object from a CapturedValue before it is passed to +// the error logger. +// TODO: CapturedError is different from CapturedValue for legacy reasons, but I +// don't think it's exposed to anyone outside FB, so we can probably change it. +function createCapturedError( + capturedValue: CapturedValue, +): CapturedError { + const source = capturedValue.source; + const boundary = capturedValue.boundary; + const stack = capturedValue.stack; + + const capturedError: CapturedError = { + componentName: source !== null ? getComponentName(source) : null, + error: capturedValue.value, + errorBoundary: boundary, + componentStack: stack !== null ? stack : '', + errorBoundaryName: null, + errorBoundaryFound: false, + willRetry: false, + }; + + if (boundary !== null) { + capturedError.errorBoundaryName = getComponentName(boundary); + // TODO: These are always the same. Why is it needed? + capturedError.errorBoundaryFound = capturedError.willRetry = + boundary.tag === ClassComponent; + } else { + capturedError.errorBoundaryName = null; + capturedError.errorBoundaryFound = capturedError.willRetry = false; + } + + return capturedError; +} + +function isError(value: mixed): boolean { + if (value instanceof Error) { + return true; + } + + // instanceof fails across realms. Check the prototype chain. + if (getPrototypeOf !== null) { + let proto = getPrototypeOf(value); + while (proto !== null) { + if (objectToString.call(proto) === '[object Error]') { + return true; + } + proto = getPrototypeOf(value); + } + return false; + } + + // If getPrototypeOf is not available, fall back to duck typing. + return ( + value !== null && + typeof value === 'object' && + typeof value.stack === 'string' && + typeof value.message === 'string' + ); +} diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 8342a028c23..f813f47364c 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -1163,6 +1163,7 @@ function ChildReconciler(shouldTrackSideEffects) { returnFiber: Fiber, currentFirstChild: Fiber | null, newChild: any, + deleteExistingChildren: boolean, expirationTime: ExpirationTime, ): Fiber | null { // This function is not recursive. @@ -1182,6 +1183,14 @@ function ChildReconciler(shouldTrackSideEffects) { newChild = newChild.props.children; } + if (deleteExistingChildren) { + // Schedule all the existing children for deletion. This has the + // effect of re-mounting children even if their identity matches, + // as if all the keys changed. + deleteRemainingChildren(returnFiber, currentFirstChild); + currentFirstChild = null; + } + // Handle object types const isObject = typeof newChild === 'object' && newChild !== null; diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 9d7c305d6fc..3d2a6c47311 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -270,11 +270,6 @@ export function createWorkInProgress( // We already have an alternate. // Reset the effect tag. workInProgress.effectTag = NoEffect; - - // The effect list is no longer valid. - workInProgress.nextEffect = null; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; } workInProgress.expirationTime = expirationTime; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 42f53a28a0a..0eba32ca24b 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -13,6 +13,7 @@ import type {HostContext} from './ReactFiberHostContext'; import type {HydrationContext} from './ReactFiberHydrationContext'; import type {FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {CapturedValue} from './ReactCapturedValue'; import { IndeterminateComponent, @@ -31,11 +32,11 @@ import { PerformedWork, Placement, ContentReset, - Err, Ref, } from 'shared/ReactTypeOfSideEffect'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; import {debugRenderPhaseSideEffects} from 'shared/ReactFeatureFlags'; +import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook'; import invariant from 'fbjs/lib/invariant'; import getComponentName from 'shared/getComponentName'; import warning from 'fbjs/lib/warning'; @@ -57,7 +58,9 @@ import { pushTopLevelContextObject, invalidateContextProvider, } from './ReactFiberContext'; + import {NoWork, Never} from './ReactFiberExpirationTime'; +import {logError} from './ReactCapturedValue'; let warnedAboutStatelessRefs; @@ -71,6 +74,7 @@ export default function( hydrationContext: HydrationContext, scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, computeExpirationForFiber: (fiber: Fiber) => ExpirationTime, + markUncaughtError: (error: mixed) => void, ) { const { shouldSetTextContent, @@ -90,7 +94,7 @@ export default function( adoptClassInstance, constructClassInstance, mountClassInstance, - // resumeMountClassInstance, + resumeMountClassInstance, updateClassInstance, } = ReactFiberClassComponent( scheduleWork, @@ -105,6 +109,7 @@ export default function( current, workInProgress, nextChildren, + false, workInProgress.expirationTime, ); } @@ -113,6 +118,7 @@ export default function( current, workInProgress, nextChildren, + deleteExistingChildren, renderExpirationTime, ) { if (current === null) { @@ -124,6 +130,7 @@ export default function( workInProgress, null, nextChildren, + deleteExistingChildren, renderExpirationTime, ); } else { @@ -137,6 +144,7 @@ export default function( workInProgress, current.child, nextChildren, + deleteExistingChildren, renderExpirationTime, ); } @@ -204,16 +212,30 @@ export default function( function updateClassComponent( current: Fiber | null, workInProgress: Fiber, + capturedValues: Array> | null, renderExpirationTime: ExpirationTime, ) { // Push context providers early to prevent context stack mismatches. // During mounting we don't know the child context yet as the instance doesn't exist. // We will invalidate the child context in finishClassComponent() right after rendering. const hasContext = pushContextProvider(workInProgress); + const instance = workInProgress.stateNode; + + let didCaptureError = false; + if (capturedValues !== null) { + didCaptureError = true; + invariant(instance !== null, 'Expected class to have an instance.'); + // TODO: Pattern matching. Check that this is an error. + const capturedValue: CapturedValue = (capturedValues[0]: any); + if (capturedValue.isError) { + logError(capturedValue); + instance.componentDidCatch(capturedValue.value); + } + } let shouldUpdate; if (current === null) { - if (!workInProgress.stateNode) { + if (instance === null) { // In the initial pass we might need to construct the instance. constructClassInstance(workInProgress, workInProgress.pendingProps); mountClassInstance(workInProgress, renderExpirationTime); @@ -229,9 +251,11 @@ export default function( shouldUpdate = true; } else { - invariant(false, 'Resuming work not yet implemented.'); // In a resume, we'll already have an instance we can reuse. - // shouldUpdate = resumeMountClassInstance(workInProgress, renderExpirationTime); + shouldUpdate = resumeMountClassInstance( + workInProgress, + renderExpirationTime, + ); } } else { shouldUpdate = updateClassInstance( @@ -240,11 +264,25 @@ export default function( renderExpirationTime, ); } + + const updateQueue = workInProgress.updateQueue; + if (updateQueue !== null && updateQueue.capturedValues !== null) { + // We already called componentDidCatch inside updateClassInstance. + // We're checking here again so we can grab the values off the + // update queue. + // TODO: Refactor class components. + capturedValues = updateQueue.capturedValues; + updateQueue.capturedValues = null; + shouldUpdate = true; + didCaptureError = true; + } return finishClassComponent( current, workInProgress, shouldUpdate, hasContext, + didCaptureError, + renderExpirationTime, ); } @@ -253,11 +291,13 @@ export default function( workInProgress: Fiber, shouldUpdate: boolean, hasContext: boolean, + didCaptureError: boolean, + renderExpirationTime: ExpirationTime, ) { // Refs should update even if shouldComponentUpdate returns false markRef(current, workInProgress); - if (!shouldUpdate) { + if (!shouldUpdate && !didCaptureError) { // Context providers should defer to sCU for rendering if (hasContext) { invalidateContextProvider(workInProgress, false); @@ -286,7 +326,13 @@ export default function( } // React DevTools reads this flag. workInProgress.effectTag |= PerformedWork; - reconcileChildren(current, workInProgress, nextChildren); + reconcileChildrenAtExpirationTime( + current, + workInProgress, + nextChildren, + didCaptureError, + renderExpirationTime, + ); // Memoize props and state using the values we just used to render. // TODO: Restructure so we never read values from the instance. memoizeState(workInProgress, instance.state); @@ -315,9 +361,53 @@ export default function( pushHostContainer(workInProgress, root.containerInfo); } - function updateHostRoot(current, workInProgress, renderExpirationTime) { + function unmountFailedRoot( + current, + workInProgress, + capturedValues, + renderExpirationTime, + ) { + const capturedValue: CapturedValue = (capturedValues[0]: any); + if (capturedValue.isError) { + logError(capturedValue); + } else { + capturedValue.isError = true; + const source = capturedValue.source; + if (source !== null) { + capturedValue.stack = getStackAddendumByWorkInProgressFiber(source); + } + } + const error = capturedValue.value; + markUncaughtError(error); + + const didError = true; + reconcileChildrenAtExpirationTime( + current, + workInProgress, + null, + didError, + renderExpirationTime, + ); + return null; + } + + function updateHostRoot( + current, + workInProgress, + capturedValues, + renderExpirationTime, + ) { pushHostRootContext(workInProgress); - const updateQueue = workInProgress.updateQueue; + if (capturedValues !== null) { + return unmountFailedRoot( + current, + workInProgress, + capturedValues, + renderExpirationTime, + ); + } + + let updateQueue = workInProgress.updateQueue; if (updateQueue !== null) { const prevState = workInProgress.memoizedState; const state = processUpdateQueue( @@ -328,6 +418,18 @@ export default function( null, renderExpirationTime, ); + updateQueue = workInProgress.updateQueue; + if (updateQueue !== null && updateQueue.capturedValues !== null) { + capturedValues = updateQueue.capturedValues; + updateQueue.capturedValues = null; + memoizeState(workInProgress, state); + return unmountFailedRoot( + current, + workInProgress, + capturedValues, + renderExpirationTime, + ); + } if (prevState === state) { // If the state is the same as before, that's a bailout because we had // no work that expires at this time. @@ -359,6 +461,7 @@ export default function( workInProgress, null, element, + false, renderExpirationTime, ); } else { @@ -489,7 +592,14 @@ export default function( const hasContext = pushContextProvider(workInProgress); adoptClassInstance(workInProgress, value); mountClassInstance(workInProgress, renderExpirationTime); - return finishClassComponent(current, workInProgress, true, hasContext); + return finishClassComponent( + current, + workInProgress, + true, + hasContext, + false, + renderExpirationTime, + ); } else { // Proceed under the assumption that this is a functional component workInProgress.tag = FunctionalComponent; @@ -554,6 +664,7 @@ export default function( workInProgress, workInProgress.stateNode, nextChildren, + false, renderExpirationTime, ); } else { @@ -561,6 +672,7 @@ export default function( workInProgress, workInProgress.stateNode, nextChildren, + false, renderExpirationTime, ); } @@ -595,6 +707,7 @@ export default function( workInProgress, null, nextChildren, + false, renderExpirationTime, ); memoizeProps(workInProgress, nextChildren); @@ -686,8 +799,14 @@ export default function( function beginWork( current: Fiber | null, workInProgress: Fiber, + capturedValues: Array> | null, renderExpirationTime: ExpirationTime, ): Fiber | null { + // The effect list is no longer valid. + workInProgress.nextEffect = null; + workInProgress.firstEffect = null; + workInProgress.lastEffect = null; + if ( workInProgress.expirationTime === NoWork || workInProgress.expirationTime > renderExpirationTime @@ -708,10 +827,16 @@ export default function( return updateClassComponent( current, workInProgress, + capturedValues, renderExpirationTime, ); case HostRoot: - return updateHostRoot(current, workInProgress, renderExpirationTime); + return updateHostRoot( + current, + workInProgress, + capturedValues, + renderExpirationTime, + ); case HostComponent: return updateHostComponent( current, @@ -751,73 +876,7 @@ export default function( } } - function beginFailedWork( - current: Fiber | null, - workInProgress: Fiber, - renderExpirationTime: ExpirationTime, - ) { - // Push context providers here to avoid a push/pop context mismatch. - switch (workInProgress.tag) { - case ClassComponent: - pushContextProvider(workInProgress); - break; - case HostRoot: - pushHostRootContext(workInProgress); - break; - default: - invariant( - false, - 'Invalid type of work. This error is likely caused by a bug in React. ' + - 'Please file an issue.', - ); - } - - // Add an error effect so we can handle the error during the commit phase - workInProgress.effectTag |= Err; - - // This is a weird case where we do "resume" work — work that failed on - // our first attempt. Because we no longer have a notion of "progressed - // deletions," reset the child to the current child to make sure we delete - // it again. TODO: Find a better way to handle this, perhaps during a more - // general overhaul of error handling. - if (current === null) { - workInProgress.child = null; - } else if (workInProgress.child !== current.child) { - workInProgress.child = current.child; - } - - if ( - workInProgress.expirationTime === NoWork || - workInProgress.expirationTime > renderExpirationTime - ) { - return bailoutOnLowPriority(current, workInProgress); - } - - // If we don't bail out, we're going be recomputing our children so we need - // to drop our effect list. - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; - - // Unmount the current children as if the component rendered null - const nextChildren = null; - reconcileChildrenAtExpirationTime( - current, - workInProgress, - nextChildren, - renderExpirationTime, - ); - - if (workInProgress.tag === ClassComponent) { - const instance = workInProgress.stateNode; - workInProgress.memoizedProps = instance.props; - workInProgress.memoizedState = instance.state; - } - - return workInProgress.child; - } - return { beginWork, - beginFailedWork, }; } diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 75ddf381699..3ac8ef478c0 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -9,6 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {CapturedValue} from './ReactCapturedValue'; import {Update} from 'shared/ReactTypeOfSideEffect'; import { @@ -36,6 +37,7 @@ import { processUpdateQueue, } from './ReactFiberUpdateQueue'; import {hasContextChanged} from './ReactFiberContext'; +import {logError} from './ReactCapturedValue'; const fakeInternalInstance = {}; const isArray = Array.isArray; @@ -100,7 +102,8 @@ export default function( callback, isReplace: false, isForced: false, - nextCallback: null, + isCapture: false, + capturedValue: null, next: null, }; insertUpdateIntoFiber(fiber, update); @@ -119,7 +122,8 @@ export default function( callback, isReplace: true, isForced: false, - nextCallback: null, + isCapture: false, + capturedValue: null, next: null, }; insertUpdateIntoFiber(fiber, update); @@ -138,7 +142,8 @@ export default function( callback, isReplace: false, isForced: true, - nextCallback: null, + isCapture: false, + capturedValue: null, next: null, }; insertUpdateIntoFiber(fiber, update); @@ -435,6 +440,16 @@ export default function( } } + function callComponentDidCatch(instance: any, capturedValues: Array) { + for (let i = 0; i < capturedValues.length; i++) { + const capturedValue: CapturedValue = (capturedValues[i]: any); + // For now, we can assume these are all errors. + logError(capturedValue); + const error = capturedValue.value; + instance.componentDidCatch(error); + } + } + // Invokes the mount life-cycles on a previously never rendered instance. function mountClassInstance( workInProgress: Fiber, @@ -486,110 +501,131 @@ export default function( } } - // Called on a preexisting class instance. Returns false if a resumed render - // could be reused. - // function resumeMountClassInstance( - // workInProgress: Fiber, - // priorityLevel: PriorityLevel, - // ): boolean { - // const instance = workInProgress.stateNode; - // resetInputPointers(workInProgress, instance); - - // let newState = workInProgress.memoizedState; - // let newProps = workInProgress.pendingProps; - // if (!newProps) { - // // If there isn't any new props, then we'll reuse the memoized props. - // // This could be from already completed work. - // newProps = workInProgress.memoizedProps; - // invariant( - // newProps != null, - // 'There should always be pending or memoized props. This error is ' + - // 'likely caused by a bug in React. Please file an issue.', - // ); - // } - // const newUnmaskedContext = getUnmaskedContext(workInProgress); - // const newContext = getMaskedContext(workInProgress, newUnmaskedContext); - - // const oldContext = instance.context; - // const oldProps = workInProgress.memoizedProps; - - // if ( - // typeof instance.componentWillReceiveProps === 'function' && - // (oldProps !== newProps || oldContext !== newContext) - // ) { - // callComponentWillReceiveProps( - // workInProgress, - // instance, - // newProps, - // newContext, - // ); - // } - - // // Process the update queue before calling shouldComponentUpdate - // const updateQueue = workInProgress.updateQueue; - // if (updateQueue !== null) { - // newState = processUpdateQueue( - // workInProgress, - // updateQueue, - // instance, - // newState, - // newProps, - // priorityLevel, - // ); - // } - - // // TODO: Should we deal with a setState that happened after the last - // // componentWillMount and before this componentWillMount? Probably - // // unsupported anyway. - - // if ( - // !checkShouldComponentUpdate( - // workInProgress, - // workInProgress.memoizedProps, - // newProps, - // workInProgress.memoizedState, - // newState, - // newContext, - // ) - // ) { - // // Update the existing instance's state, props, and context pointers even - // // though we're bailing out. - // instance.props = newProps; - // instance.state = newState; - // instance.context = newContext; - // return false; - // } - - // // Update the input pointers now so that they are correct when we call - // // componentWillMount - // instance.props = newProps; - // instance.state = newState; - // instance.context = newContext; - - // if (typeof instance.componentWillMount === 'function') { - // callComponentWillMount(workInProgress, instance); - // // componentWillMount may have called setState. Process the update queue. - // const newUpdateQueue = workInProgress.updateQueue; - // if (newUpdateQueue !== null) { - // newState = processUpdateQueue( - // workInProgress, - // newUpdateQueue, - // instance, - // newState, - // newProps, - // priorityLevel, - // ); - // } - // } - - // if (typeof instance.componentDidMount === 'function') { - // workInProgress.effectTag |= Update; - // } - - // instance.state = newState; - - // return true; - // } + function resumeMountClassInstance( + workInProgress: Fiber, + renderExpirationTime: ExpirationTime, + ): boolean { + const instance = workInProgress.stateNode; + resetInputPointers(workInProgress, instance); + + const oldProps = workInProgress.memoizedProps; + const newProps = workInProgress.pendingProps; + const oldContext = instance.context; + const newUnmaskedContext = getUnmaskedContext(workInProgress); + const newContext = getMaskedContext(workInProgress, newUnmaskedContext); + + // Note: During these life-cycles, instance.props/instance.state are what + // ever the previously attempted to render - not the "current". However, + // during componentDidUpdate we pass the "current" props. + + if ( + typeof instance.componentWillReceiveProps === 'function' && + (oldProps !== newProps || oldContext !== newContext) + ) { + callComponentWillReceiveProps( + workInProgress, + instance, + newProps, + newContext, + ); + } + + // Compute the next state using the memoized state and the update queue. + const oldState = workInProgress.memoizedState; + // TODO: Previous state can be null. + let newState; + if (workInProgress.updateQueue !== null) { + newState = processUpdateQueue( + null, + workInProgress, + workInProgress.updateQueue, + instance, + newProps, + renderExpirationTime, + ); + + let updateQueue = workInProgress.updateQueue; + if (updateQueue !== null && updateQueue.capturedValues !== null) { + const capturedValues = updateQueue.capturedValues; + // Don't remove these from the update queue yet. We need them in + // finishClassComponent. Do the reset there. + // TODO: This is awkward. Refactor class components. + // updateQueue.capturedValues = null; + callComponentDidCatch(instance, capturedValues); + newState = processUpdateQueue( + null, + workInProgress, + updateQueue, + instance, + newProps, + renderExpirationTime, + ); + } + } else { + newState = oldState; + } + + if ( + oldProps === newProps && + oldState === newState && + !hasContextChanged() && + !( + workInProgress.updateQueue !== null && + workInProgress.updateQueue.hasForceUpdate + ) + ) { + // If an update was already in progress, we should schedule an Update + // effect even though we're bailing out, so that cWU/cDU are called. + if (typeof instance.componentDidUpdate === 'function') { + workInProgress.effectTag |= Update; + } + return false; + } + + const shouldUpdate = checkShouldComponentUpdate( + workInProgress, + oldProps, + newProps, + oldState, + newState, + newContext, + ); + + if (shouldUpdate) { + if (typeof instance.componentWillMount === 'function') { + startPhaseTimer(workInProgress, 'componentWillMount'); + instance.componentWillMount(newProps, newState, newContext); + stopPhaseTimer(); + + // Simulate an async bailout/interruption by invoking lifecycle twice. + if (debugRenderPhaseSideEffects) { + instance.componentWillMount(newProps, newState, newContext); + } + } + if (typeof instance.componentDidMount === 'function') { + workInProgress.effectTag |= Update; + } + } else { + // If an update was already in progress, we should schedule an Update + // effect even though we're bailing out, so that cWU/cDU are called. + if (typeof instance.componentDidMount === 'function') { + workInProgress.effectTag |= Update; + } + + // If shouldComponentUpdate returned false, we should still update the + // memoized props/state to indicate that this work can be reused. + memoizeProps(workInProgress, newProps); + memoizeState(workInProgress, newState); + } + + // Update the existing instance's state, props, and context pointers even + // if shouldComponentUpdate returns false. + instance.props = newProps; + instance.state = newState; + instance.context = newContext; + + return shouldUpdate; + } // Invokes the update life-cycles and returns false if it shouldn't rerender. function updateClassInstance( @@ -635,6 +671,24 @@ export default function( newProps, renderExpirationTime, ); + + let updateQueue = workInProgress.updateQueue; + if (updateQueue !== null && updateQueue.capturedValues !== null) { + const capturedValues = updateQueue.capturedValues; + // Don't remove these from the update queue yet. We need them in + // finishClassComponent. Do the reset there. + // TODO: This is awkward. Refactor class components. + // updateQueue.capturedValues = null; + callComponentDidCatch(instance, capturedValues); + newState = processUpdateQueue( + null, + workInProgress, + updateQueue, + instance, + newProps, + renderExpirationTime, + ); + } } else { newState = oldState; } @@ -715,7 +769,7 @@ export default function( adoptClassInstance, constructClassInstance, mountClassInstance, - // resumeMountClassInstance, + resumeMountClassInstance, updateClassInstance, }; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 40bb9ea4f46..a3fcaab15a0 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -143,6 +143,7 @@ export default function( workInProgress, currentFirstChild, nextChildren, + false, renderExpirationTime, ); return workInProgress.child; @@ -407,7 +408,6 @@ export default function( fiberRoot.context = fiberRoot.pendingContext; fiberRoot.pendingContext = null; } - if (current === null || current.child === null) { // If we hydrated, pop so that we can delete any remaining children // that weren't hydrated. diff --git a/packages/react-reconciler/src/ReactFiberErrorDialog.js b/packages/react-reconciler/src/ReactFiberErrorDialog.js index 8459be5bad8..60863514d8a 100644 --- a/packages/react-reconciler/src/ReactFiberErrorDialog.js +++ b/packages/react-reconciler/src/ReactFiberErrorDialog.js @@ -7,7 +7,7 @@ * @flow */ -import type {CapturedError} from './ReactFiberScheduler'; +import type {CapturedError} from './ReactCapturedValue'; // This module is forked in different environments. // By default, return `true` to log errors to the console. diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index d9bf2bb950c..1a15878262d 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -7,7 +7,7 @@ * @flow */ -import type {CapturedError} from './ReactFiberScheduler'; +import type {CapturedError} from './ReactCapturedValue'; import {showErrorDialog} from './ReactFiberErrorDialog'; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index ef1eb8cf008..e80c1cdb968 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -344,6 +344,8 @@ export default function( callback, isReplace: false, isForced: false, + isCapture: false, + capturedValue: null, next: null, }; insertUpdateIntoFiber(current, update); diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index f8c499654c7..dc9560734e3 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -12,11 +12,12 @@ import type {Fiber} from './ReactFiber'; import type {FiberRoot, Batch} from './ReactFiberRoot'; import type {HydrationContext} from './ReactFiberHydrationContext'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {CapturedValue} from './ReactCapturedValue'; -import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook'; import ReactErrorUtils from 'shared/ReactErrorUtils'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; import { + NoEffect, PerformedWork, Placement, Update, @@ -64,7 +65,6 @@ import { } from './ReactDebugFiberPerf'; import {popContextProvider} from './ReactFiberContext'; import {reset} from './ReactFiberStack'; -import {logCapturedError} from './ReactFiberErrorLogger'; import {createWorkInProgress} from './ReactFiber'; import {onCommitRoot} from './ReactFiberDevToolsHook'; import { @@ -76,8 +76,12 @@ import { computeExpirationBucket, } from './ReactFiberExpirationTime'; import {AsyncUpdates} from './ReactTypeOfInternalContext'; -import {getUpdateExpirationTime} from './ReactFiberUpdateQueue'; +import { + getUpdateExpirationTime, + insertUpdateIntoFiber, +} from './ReactFiberUpdateQueue'; import {resetContext} from './ReactFiberContext'; +import {createCapturedValue} from './ReactCapturedValue'; const { invokeGuardedCallback, @@ -85,20 +89,6 @@ const { clearCaughtError, } = ReactErrorUtils; -export type CapturedError = { - componentName: ?string, - componentStack: string, - error: mixed, - errorBoundary: ?Object, - errorBoundaryFound: boolean, - errorBoundaryName: string | null, - willRetry: boolean, -}; - -export type HandleErrorInfo = { - componentStack: string, -}; - let didWarnAboutStateTransition; let didWarnSetStateChildContext; let warnAboutUpdateOnUnmounted; @@ -162,12 +152,13 @@ export default function( config, ); const {popHostContainer, popHostContext, resetHostContainer} = hostContext; - const {beginWork, beginFailedWork} = ReactFiberBeginWork( + const {beginWork} = ReactFiberBeginWork( config, hostContext, hydrationContext, scheduleWork, computeExpirationForFiber, + markUncaughtError, ); const {completeWork} = ReactFiberCompleteWork( config, @@ -182,7 +173,7 @@ export default function( commitLifeCycles, commitAttachRef, commitDetachRef, - } = ReactFiberCommitWork(config, captureError); + } = ReactFiberCommitWork(config, onCommitPhaseError); const { now, scheduleDeferredCallback, @@ -215,21 +206,13 @@ export default function( // The next fiber with an effect that we're currently committing. let nextEffect: Fiber | null = null; - // Keep track of which fibers have captured an error that need to be handled. - // Work is removed from this collection after componentDidCatch is called. - let capturedErrors: Map | null = null; - // Keep track of which fibers have failed during the current batch of work. - // This is a different set than capturedErrors, because it is not reset until - // the end of the batch. This is needed to propagate errors correctly if a - // subtree fails more than once. - let failedBoundaries: Set | null = null; - // Error boundaries that captured an error during the current commit. - let commitPhaseBoundaries: Set | null = null; + let hasUncaughtError: boolean = false; let firstUncaughtError: mixed | null = null; - let didFatal: boolean = false; let isCommitting: boolean = false; - let isUnmounting: boolean = false; + + let thrownValues: Array> | null = null; + let capturedValues: Array> | null = null; // Used for performance tracking. let interruptedBy: Fiber | null = null; @@ -240,6 +223,9 @@ export default function( // Reset the cursors resetContext(); resetHostContainer(); + + thrownValues = null; + capturedValues = null; } function commitAllHostEffects() { @@ -296,9 +282,7 @@ export default function( break; } case Deletion: { - isUnmounting = true; commitDeletion(nextEffect); - isUnmounting = false; break; } } @@ -325,11 +309,6 @@ export default function( commitAttachRef(nextEffect); } - if (effectTag & Err) { - recordEffect(); - commitErrorHandling(nextEffect); - } - const next = nextEffect.nextEffect; // Ensure that we clean these up so that we don't accidentally keep them. // I'm not actually sure this matters because we can't reset firstEffect @@ -343,10 +322,6 @@ export default function( } function commitRoot(finishedWork: Fiber): ExpirationTime { - // We keep track of this so that captureError can collect any boundaries - // that capture an error during the commit phase. The reason these aren't - // local to this function is because errors that occur during cWU are - // captured elsewhere, to prevent the unmount from being interrupted. isWorking = true; isCommitting = true; startCommitTimer(); @@ -410,7 +385,7 @@ export default function( 'Should have next effect. This error is likely caused by a bug ' + 'in React. Please file an issue.', ); - captureError(nextEffect, error); + onCommitPhaseError(nextEffect, error); // Clean-up if (nextEffect !== null) { nextEffect = nextEffect.nextEffect; @@ -456,7 +431,7 @@ export default function( 'Should have next effect. This error is likely caused by a bug ' + 'in React. Please file an issue.', ); - captureError(nextEffect, error); + onCommitPhaseError(nextEffect, error); if (nextEffect !== null) { nextEffect = nextEffect.nextEffect; } @@ -474,26 +449,7 @@ export default function( ReactFiberInstrumentation.debugTool.onCommitWork(finishedWork); } - // If we caught any errors during this commit, schedule their boundaries - // to update. - if (commitPhaseBoundaries) { - commitPhaseBoundaries.forEach(scheduleErrorRecovery); - commitPhaseBoundaries = null; - } - - if (firstUncaughtError !== null) { - const error = firstUncaughtError; - firstUncaughtError = null; - onUncaughtError(error); - } - const remainingTime = root.current.expirationTime; - - if (remainingTime === NoWork) { - capturedErrors = null; - failedBoundaries = null; - } - return remainingTime; } @@ -527,7 +483,43 @@ export default function( workInProgress.expirationTime = newExpirationTime; } - function completeUnitOfWork(workInProgress: Fiber): Fiber | null { + function captureThrownValues(): boolean { + if (thrownValues === null) { + return false; + } + capturedValues = thrownValues; + // Reset the list of thrown values, now that they've been captured. + thrownValues = null; + return true; + } + + function captureErrors(): boolean { + if (thrownValues === null) { + return false; + } + let errors = null; + for (let i = 0; i < thrownValues.length; i++) { + const value = thrownValues[i]; + if (value.isError) { + thrownValues.splice(i, 1); + if (errors === null) { + errors = [value]; + } else { + errors.push(value); + } + } + } + if (thrownValues.length === 0) { + thrownValues = null; + } + capturedValues = errors; + return true; + } + + function unwindUnitOfWork(workInProgress: Fiber): Fiber | null { + // Attempt to complete the current unit of work, then move to the + // next sibling. If there are no more siblings, return to the + // parent fiber. while (true) { // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here @@ -537,11 +529,60 @@ export default function( if (__DEV__) { ReactDebugCurrentFiber.setCurrentFiber(workInProgress); } - const next = completeWork( - current, - workInProgress, - nextRenderExpirationTime, - ); + + let next; + if (thrownValues === null) { + // This fiber completed. + next = completeWork(current, workInProgress, nextRenderExpirationTime); + if (workInProgress.effectTag & Err) { + // Restarting an error boundary + stopFailedWorkTimer(workInProgress); + } else { + stopWorkTimer(workInProgress); + } + resetExpirationTime(workInProgress, nextRenderExpirationTime); + } else { + // This fiber did not complete because something threw. Pop values off + // the stack without entering the complete phase. If this is a boundary, + // capture values if possible. + popContexts(workInProgress); + switch (workInProgress.tag) { + case ClassComponent: { + const instance = workInProgress.stateNode; + if ( + (workInProgress.effectTag & Err) === NoEffect && + instance !== null && + typeof instance.componentDidCatch === 'function' + ) { + const didCapture = captureErrors(); + if (didCapture) { + workInProgress.effectTag |= Err; + // Render the boundary again + next = workInProgress; + } else { + next = null; + } + } else { + next = null; + } + break; + } + case HostRoot: + const didCapture = captureThrownValues(); + if (didCapture) { + // Render the boundary again + next = workInProgress; + } else { + next = null; + } + break; + default: + next = null; + } + // Because this fiber did not complete, don't reset its expiration time. + stopWorkTimer(workInProgress); + } + if (__DEV__) { ReactDebugCurrentFiber.resetCurrentFiber(); } @@ -549,8 +590,6 @@ export default function( const returnFiber = workInProgress.return; const siblingFiber = workInProgress.sibling; - resetExpirationTime(workInProgress, nextRenderExpirationTime); - if (next !== null) { stopWorkTimer(workInProgress); if (__DEV__ && ReactFiberInstrumentation.debugTool) { @@ -594,7 +633,6 @@ export default function( } } - stopWorkTimer(workInProgress); if (__DEV__ && ReactFiberInstrumentation.debugTool) { ReactFiberInstrumentation.debugTool.onCompleteWork(workInProgress); } @@ -633,41 +671,13 @@ export default function( ReactDebugCurrentFiber.setCurrentFiber(workInProgress); } - let next = beginWork(current, workInProgress, nextRenderExpirationTime); - if (__DEV__) { - ReactDebugCurrentFiber.resetCurrentFiber(); - } - if (__DEV__ && ReactFiberInstrumentation.debugTool) { - ReactFiberInstrumentation.debugTool.onBeginWork(workInProgress); - } - - if (next === null) { - // If this doesn't spawn new work, complete the current work. - next = completeUnitOfWork(workInProgress); - } - - ReactCurrentOwner.current = null; - - return next; - } - - function performFailedUnitOfWork(workInProgress: Fiber): Fiber | null { - // The current, flushed, state of this fiber is the alternate. - // Ideally nothing should rely on this, but relying on it here - // means that we don't need an additional field on the work in - // progress. - const current = workInProgress.alternate; - - // See if beginning this work spawns more work. - startWorkTimer(workInProgress); - if (__DEV__) { - ReactDebugCurrentFiber.setCurrentFiber(workInProgress); - } - let next = beginFailedWork( + let next = beginWork( current, workInProgress, + capturedValues, nextRenderExpirationTime, ); + capturedValues = null; if (__DEV__) { ReactDebugCurrentFiber.resetCurrentFiber(); } @@ -677,7 +687,7 @@ export default function( if (next === null) { // If this doesn't spawn new work, complete the current work. - next = completeUnitOfWork(workInProgress); + next = unwindUnitOfWork(workInProgress); } ReactCurrentOwner.current = null; @@ -686,14 +696,6 @@ export default function( } function workLoop(expirationTime: ExpirationTime) { - if (capturedErrors !== null) { - // If there are unhandled errors, switch to the slow work loop. - // TODO: How to avoid this check in the fast path? Maybe the renderer - // could keep track of which roots have unhandled errors and call a - // forked version of renderRoot. - slowWorkLoopThatChecksForFailedWork(expirationTime); - return; - } if ( nextRenderExpirationTime === NoWork || nextRenderExpirationTime > expirationTime @@ -714,59 +716,6 @@ export default function( } } - function slowWorkLoopThatChecksForFailedWork(expirationTime: ExpirationTime) { - if ( - nextRenderExpirationTime === NoWork || - nextRenderExpirationTime > expirationTime - ) { - return; - } - - if (nextRenderExpirationTime <= mostRecentCurrentTime) { - // Flush all expired work. - while (nextUnitOfWork !== null) { - if (hasCapturedError(nextUnitOfWork)) { - // Use a forked version of performUnitOfWork - nextUnitOfWork = performFailedUnitOfWork(nextUnitOfWork); - } else { - nextUnitOfWork = performUnitOfWork(nextUnitOfWork); - } - } - } else { - // Flush asynchronous work until the deadline runs out of time. - while (nextUnitOfWork !== null && !shouldYield()) { - if (hasCapturedError(nextUnitOfWork)) { - // Use a forked version of performUnitOfWork - nextUnitOfWork = performFailedUnitOfWork(nextUnitOfWork); - } else { - nextUnitOfWork = performUnitOfWork(nextUnitOfWork); - } - } - } - } - - function renderRootCatchBlock( - root: FiberRoot, - failedWork: Fiber, - boundary: Fiber, - expirationTime: ExpirationTime, - ) { - // We're going to restart the error boundary that captured the error. - // Conceptually, we're unwinding the stack. We need to unwind the - // context stack, too. - unwindContexts(failedWork, boundary); - - // Restart the error boundary using a forked version of - // performUnitOfWork that deletes the boundary's children. The entire - // failed subree will be unmounted. During the commit phase, a special - // lifecycle method is called on the error boundary, which triggers - // a re-render. - nextUnitOfWork = performFailedUnitOfWork(boundary); - - // Continue working. - workLoop(expirationTime); - } - function renderRoot( root: FiberRoot, expirationTime: ExpirationTime, @@ -802,339 +751,152 @@ export default function( startWorkLoopTimer(nextUnitOfWork); - let didError = false; - let error = null; - if (__DEV__) { - invokeGuardedCallback(null, workLoop, null, expirationTime); - if (hasCaughtError()) { - didError = true; - error = clearCaughtError(); - } - } else { - try { - workLoop(expirationTime); - } catch (e) { - didError = true; - error = e; + let didThrow; + let thrownValue; + do { + didThrow = false; + thrownValue = null; + + if (__DEV__) { + invokeGuardedCallback(null, workLoop, null, expirationTime); + if (hasCaughtError()) { + didThrow = true; + thrownValue = clearCaughtError(); + } + } else { + try { + workLoop(expirationTime); + } catch (e) { + didThrow = true; + thrownValue = e; + } } - } - // An error was thrown during the render phase. - while (didError) { - if (didFatal) { - // This was a fatal error. Don't attempt to recover from it. - firstUncaughtError = error; + if (!didThrow) { break; } - const failedWork = nextUnitOfWork; - if (failedWork === null) { - // An error was thrown but there's no current unit of work. This can - // happen during the commit phase if there's a bug in the renderer. - didFatal = true; - continue; + if (nextUnitOfWork === null) { + // Should have a nextUnitOfWork here. + hasUncaughtError = true; + firstUncaughtError = thrownValue; + break; } - // "Capture" the error by finding the nearest boundary. If there is no - // error boundary, we use the root. - const boundary = captureError(failedWork, error); - invariant( - boundary !== null, - 'Should have found an error boundary. This error is likely ' + - 'caused by a bug in React. Please file an issue.', - ); - - if (didFatal) { - // The error we just captured was a fatal error. This happens - // when the error propagates to the root more than once. - continue; + const sourceFiber: Fiber = nextUnitOfWork; + const capturedValue = createCapturedValue(thrownValue, sourceFiber); + if (thrownValues === null) { + thrownValues = [capturedValue]; + } else { + thrownValues.push(capturedValue); } - didError = false; - error = null; - if (__DEV__) { - invokeGuardedCallback( - null, - renderRootCatchBlock, - null, - root, - failedWork, - boundary, - expirationTime, - ); - if (hasCaughtError()) { - didError = true; - error = clearCaughtError(); - continue; - } + // Move to next sibling, or return to parent + popContexts(sourceFiber); + if (sourceFiber.sibling !== null) { + nextUnitOfWork = sourceFiber.sibling; + } else if (sourceFiber.return !== null) { + nextUnitOfWork = unwindUnitOfWork(sourceFiber.return); } else { - try { - renderRootCatchBlock(root, failedWork, boundary, expirationTime); - error = null; - } catch (e) { - didError = true; - error = e; - continue; - } + // The root failed to render. This is a fatal error. + hasUncaughtError = true; + firstUncaughtError = thrownValue; + break; } - // We're finished working. Exit the error loop. - break; - } + } while (true); - const uncaughtError = firstUncaughtError; + if (hasUncaughtError) { + onUncaughtError(firstUncaughtError); + hasUncaughtError = false; + firstUncaughtError = null; + // Set this to null to indicate there's no more work left. + // That way the stack is reset next time we work on this root. + nextUnitOfWork = null; + } // We're done performing work. Time to clean up. stopWorkLoopTimer(interruptedBy); interruptedBy = null; isWorking = false; - didFatal = false; - firstUncaughtError = null; - - if (uncaughtError !== null) { - onUncaughtError(uncaughtError); - } return root.isReadyForCommit ? root.current.alternate : null; } - // Returns the boundary that captured the error, or null if the error is ignored - function captureError(failedWork: Fiber, error: mixed): Fiber | null { - // It is no longer valid because we exited the user code. - ReactCurrentOwner.current = null; - if (__DEV__) { - ReactDebugCurrentFiber.resetCurrentFiber(); - } - - // Search for the nearest error boundary. - let boundary: Fiber | null = null; - - // Passed to logCapturedError() - let errorBoundaryFound: boolean = false; - let willRetry: boolean = false; - let errorBoundaryName: string | null = null; - - // Host containers are a special case. If the failed work itself is a host - // container, then it acts as its own boundary. In all other cases, we - // ignore the work itself and only search through the parents. - if (failedWork.tag === HostRoot) { - boundary = failedWork; - - if (isFailedBoundary(failedWork)) { - // If this root already failed, there must have been an error when - // attempting to unmount it. This is a worst-case scenario and - // should only be possible if there's a bug in the renderer. - didFatal = true; - } - } else { - let node = failedWork.return; - while (node !== null && boundary === null) { - if (node.tag === ClassComponent) { - const instance = node.stateNode; - if (typeof instance.componentDidCatch === 'function') { - errorBoundaryFound = true; - errorBoundaryName = getComponentName(node); - - // Found an error boundary! - boundary = node; - willRetry = true; - } - } else if (node.tag === HostRoot) { - // Treat the root like a no-op error boundary - boundary = node; - } - - if (isFailedBoundary(node)) { - // This boundary is already in a failed state. - - // If we're currently unmounting, that means this error was - // thrown while unmounting a failed subtree. We should ignore - // the error. - if (isUnmounting) { - return null; - } - - // If we're in the commit phase, we should check to see if - // this boundary already captured an error during this commit. - // This case exists because multiple errors can be thrown during - // a single commit without interruption. - if ( - commitPhaseBoundaries !== null && - (commitPhaseBoundaries.has(node) || - (node.alternate !== null && - commitPhaseBoundaries.has(node.alternate))) - ) { - // If so, we should ignore this error. - return null; - } - - // The error should propagate to the next boundary -— we keep looking. - boundary = null; - willRetry = false; - } - - node = node.return; - } - } - - if (boundary !== null) { - // Add to the collection of failed boundaries. This lets us know that - // subsequent errors in this subtree should propagate to the next boundary. - if (failedBoundaries === null) { - failedBoundaries = new Set(); - } - failedBoundaries.add(boundary); - - // This method is unsafe outside of the begin and complete phases. - // We might be in the commit phase when an error is captured. - // The risk is that the return path from this Fiber may not be accurate. - // That risk is acceptable given the benefit of providing users more context. - const componentStack = getStackAddendumByWorkInProgressFiber(failedWork); - const componentName = getComponentName(failedWork); - - // Add to the collection of captured errors. This is stored as a global - // map of errors and their component stack location keyed by the boundaries - // that capture them. We mostly use this Map as a Set; it's a Map only to - // avoid adding a field to Fiber to store the error. - if (capturedErrors === null) { - capturedErrors = new Map(); - } - - const capturedError = { - componentName, - componentStack, - error, - errorBoundary: errorBoundaryFound ? boundary.stateNode : null, - errorBoundaryFound, - errorBoundaryName, - willRetry, - }; - - capturedErrors.set(boundary, capturedError); - - try { - logCapturedError(capturedError); - } catch (e) { - // Prevent cycle if logCapturedError() throws. - // A cycle may still occur if logCapturedError renders a component that throws. - const suppressLogging = e && e.suppressReactErrorLogging; - if (!suppressLogging) { - console.error(e); - } - } - - // If we're in the commit phase, defer scheduling an update on the - // boundary until after the commit is complete - if (isCommitting) { - if (commitPhaseBoundaries === null) { - commitPhaseBoundaries = new Set(); - } - commitPhaseBoundaries.add(boundary); - } else { - // Otherwise, schedule an update now. - // TODO: Is this actually necessary during the render phase? Is it - // possible to unwind and continue rendering at the same priority, - // without corrupting internal state? - scheduleErrorRecovery(boundary); - } - return boundary; - } else if (firstUncaughtError === null) { - // If no boundary is found, we'll need to throw the error - firstUncaughtError = error; + function popContexts(workInProgress: Fiber) { + switch (workInProgress.tag) { + case ClassComponent: + popContextProvider(workInProgress); + break; + case HostComponent: + popHostContext(workInProgress); + break; + case HostRoot: + popHostContainer(workInProgress); + break; + case HostPortal: + popHostContainer(workInProgress); + break; } - return null; + stopWorkTimer(workInProgress); } - function hasCapturedError(fiber: Fiber): boolean { - // TODO: capturedErrors should store the boundary instance, to avoid needing - // to check the alternate. - return ( - capturedErrors !== null && - (capturedErrors.has(fiber) || - (fiber.alternate !== null && capturedErrors.has(fiber.alternate))) - ); + function markUncaughtError(error: mixed): void { + hasUncaughtError = true; + firstUncaughtError = error; } - function isFailedBoundary(fiber: Fiber): boolean { - // TODO: failedBoundaries should store the boundary instance, to avoid - // needing to check the alternate. - return ( - failedBoundaries !== null && - (failedBoundaries.has(fiber) || - (fiber.alternate !== null && failedBoundaries.has(fiber.alternate))) - ); + function scheduleCapture(sourceFiber, boundaryFiber, value, expirationTime) { + const capturedValue = createCapturedValue(value, sourceFiber); + capturedValue.boundary = boundaryFiber; + const update = { + expirationTime, + partialState: null, + callback: null, + isReplace: false, + isForced: false, + isCapture: true, + capturedValue, + next: null, + }; + insertUpdateIntoFiber(boundaryFiber, update); + scheduleWork(boundaryFiber, expirationTime); } - function commitErrorHandling(effectfulFiber: Fiber) { - let capturedError; - if (capturedErrors !== null) { - capturedError = capturedErrors.get(effectfulFiber); - capturedErrors.delete(effectfulFiber); - if (capturedError == null) { - if (effectfulFiber.alternate !== null) { - effectfulFiber = effectfulFiber.alternate; - capturedError = capturedErrors.get(effectfulFiber); - capturedErrors.delete(effectfulFiber); - } - } - } - + function dispatch( + sourceFiber: Fiber, + value: mixed, + expirationTime: ExpirationTime, + ) { invariant( - capturedError != null, - 'No error for given unit of work. This error is likely caused by a ' + - 'bug in React. Please file an issue.', + !isWorking || isCommitting, + 'dispatch: Cannot dispatch during the render phase.', ); - - switch (effectfulFiber.tag) { - case ClassComponent: - const instance = effectfulFiber.stateNode; - - const info: HandleErrorInfo = { - componentStack: capturedError.componentStack, - }; - - // Allow the boundary to handle the error, usually by scheduling - // an update to itself - instance.componentDidCatch(capturedError.error, info); - return; - case HostRoot: - if (firstUncaughtError === null) { - firstUncaughtError = capturedError.error; - } - return; - default: - invariant( - false, - 'Invalid type of work. This error is likely caused by a bug in ' + - 'React. Please file an issue.', - ); - } - } - - function unwindContexts(from: Fiber, to: Fiber) { - let node = from; - while (node !== null) { - switch (node.tag) { + let fiber = sourceFiber.return; + while (fiber !== null) { + switch (fiber.tag) { case ClassComponent: - popContextProvider(node); - break; - case HostComponent: - popHostContext(node); + const instance = fiber.stateNode; + if (typeof instance.componentDidCatch === 'function') { + scheduleCapture(sourceFiber, fiber, value, expirationTime); + return; + } break; case HostRoot: - popHostContainer(node); - break; - case HostPortal: - popHostContainer(node); - break; - } - if (node === to || node.alternate === to) { - stopFailedWorkTimer(node); - break; - } else { - stopWorkTimer(node); + scheduleCapture(sourceFiber, fiber, value, expirationTime); + return; } - node = node.return; + fiber = fiber.return; } + + if (sourceFiber.tag === HostRoot) { + // Error was thrown at the root. There is no parent, so the root + // itself should capture it. + scheduleCapture(sourceFiber, sourceFiber, value, expirationTime); + } + } + + function onCommitPhaseError(fiber: Fiber, error: mixed) { + return dispatch(fiber, error, Sync); } function computeAsyncExpiration() { @@ -1266,10 +1028,6 @@ export default function( } } - function scheduleErrorRecovery(fiber: Fiber) { - scheduleWorkImpl(fiber, Sync, true); - } - function recalculateCurrentTime(): ExpirationTime { // Subtract initial time so it fits inside 32bits const ms = now() - startTime; diff --git a/packages/react-reconciler/src/ReactFiberUpdateQueue.js b/packages/react-reconciler/src/ReactFiberUpdateQueue.js index a036c5d39e4..c160cbc7068 100644 --- a/packages/react-reconciler/src/ReactFiberUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberUpdateQueue.js @@ -9,6 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {CapturedValue} from './ReactCapturedValue'; import {debugRenderPhaseSideEffects} from 'shared/ReactFeatureFlags'; import {Callback as CallbackEffect} from 'shared/ReactTypeOfSideEffect'; @@ -37,6 +38,7 @@ export type Update = { callback: Callback | null, isReplace: boolean, isForced: boolean, + capturedValue: CapturedValue | null, next: Update | null, }; @@ -64,6 +66,7 @@ export type UpdateQueue = { callbackList: Array> | null, hasForceUpdate: boolean, isInitialized: boolean, + capturedValues: Array> | null, // Dev only isProcessing?: boolean, @@ -78,6 +81,7 @@ function createUpdateQueue(baseState: State): UpdateQueue { callbackList: null, hasForceUpdate: false, isInitialized: false, + capturedValues: null, }; if (__DEV__) { queue.isProcessing = false; @@ -213,6 +217,7 @@ export function processUpdateQueue( first: currentQueue.first, last: currentQueue.last, isInitialized: currentQueue.isInitialized, + capturedValues: currentQueue.capturedValues, // These fields are no longer valid because they were already committed. // Reset them. callbackList: null, @@ -304,12 +309,24 @@ export function processUpdateQueue( } callbackList.push(update); } + if (update.capturedValue !== null) { + let capturedValues = queue.capturedValues; + if (capturedValues === null) { + queue.capturedValues = [update.capturedValue]; + } else { + capturedValues.push(update.capturedValue); + } + } update = update.next; } if (queue.callbackList !== null) { workInProgress.effectTag |= CallbackEffect; - } else if (queue.first === null && !queue.hasForceUpdate) { + } else if ( + queue.first === null && + !queue.hasForceUpdate && + queue.capturedValues === null + ) { // The queue is empty. We can reset it. workInProgress.updateQueue = null; } diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js index f12056aa5dd..172d9f9ebc0 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js @@ -2717,7 +2717,9 @@ describe('ReactIncremental', () => { expect(ReactNoop.flush()).toEqual([]); }); - it('does not break with a bad Map polyfill', () => { + // We don't currently use fibers as keys. Re-enable this test if we + // ever do again. + it.skip('does not break with a bad Map polyfill', () => { const realMapSet = Map.prototype.set; function triggerCodePathThatUsesFibersAsMapKeys() { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js index 80210157e00..a4dd36985bc 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js @@ -31,6 +31,115 @@ describe('ReactIncrementalErrorHandling', () => { return {type: 'span', children: [], prop}; } + it('recovers from errors asynchronously', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + componentDidCatch(error) { + ReactNoop.yield('componentDidCatch'); + this.setState({error}); + } + render() { + if (this.state.error) { + ReactNoop.yield('ErrorBoundary (catch)'); + return ; + } + ReactNoop.yield('ErrorBoundary (try)'); + return this.props.children; + } + } + + function ErrorMessage(props) { + ReactNoop.yield('ErrorMessage'); + return ; + } + + function Indirection(props) { + ReactNoop.yield('Indirection'); + return props.children; + } + + function BadRender() { + ReactNoop.yield('throw'); + throw new Error('oops!'); + } + + ReactNoop.render( + + + + + + + + + , + ); + + ReactNoop.flushThrough([ + 'ErrorBoundary (try)', + 'Indirection', + 'Indirection', + 'Indirection', + // The error was thrown, but React ran out of time and yielded + // before recovering. + 'throw', + ]); + + // Upon resuming, componentDidCatch is called + ReactNoop.flushThrough(['componentDidCatch', 'ErrorBoundary (catch)']); + + // Flush the rest of the work + expect(ReactNoop.flush()).toEqual(['ErrorMessage']); + expect(ReactNoop.getChildren()).toEqual([span('Caught an error: oops!')]); + }); + + it('calls componentDidCatch multiple times for multiple errors', () => { + let id = 0; + class BadMount extends React.Component { + componentDidMount() { + throw new Error(`Error ${++id}`); + } + render() { + ReactNoop.yield('BadMount'); + return null; + } + } + + class ErrorBoundary extends React.Component { + state = {errorCount: 0}; + componentDidCatch(error) { + ReactNoop.yield(`componentDidCatch: ${error.message}`); + this.setState(state => ({errorCount: state.errorCount + 1})); + } + render() { + if (this.state.errorCount > 0) { + return ; + } + ReactNoop.yield('ErrorBoundary'); + return this.props.children; + } + } + + ReactNoop.render( + + + + + , + ); + + expect(ReactNoop.flush()).toEqual([ + 'ErrorBoundary', + 'BadMount', + 'BadMount', + 'BadMount', + 'componentDidCatch: Error 1', + 'componentDidCatch: Error 2', + 'componentDidCatch: Error 3', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Number of errors: 3')]); + }); + it('catches render error in a boundary during full deferred mounting', () => { class ErrorBoundary extends React.Component { state = {error: null}; @@ -61,27 +170,26 @@ describe('ReactIncrementalErrorHandling', () => { }); it('catches render error in a boundary during partial deferred mounting', () => { - const ops = []; class ErrorBoundary extends React.Component { state = {error: null}; componentDidCatch(error) { - ops.push('ErrorBoundary componentDidCatch'); + ReactNoop.yield('ErrorBoundary componentDidCatch'); this.setState({error}); } render() { if (this.state.error) { - ops.push('ErrorBoundary render error'); + ReactNoop.yield('ErrorBoundary render error'); return ( ); } - ops.push('ErrorBoundary render success'); + ReactNoop.yield('ErrorBoundary render success'); return this.props.children; } } function BrokenRender(props) { - ops.push('BrokenRender'); + ReactNoop.yield('BrokenRender'); throw new Error('Hello'); } @@ -91,13 +199,10 @@ describe('ReactIncrementalErrorHandling', () => { , ); - ReactNoop.flushDeferredPri(15); - expect(ops).toEqual(['ErrorBoundary render success']); + ReactNoop.flushThrough(['ErrorBoundary render success']); expect(ReactNoop.getChildren()).toEqual([]); - ops.length = 0; - ReactNoop.flushDeferredPri(30); - expect(ops).toEqual([ + expect(ReactNoop.flush()).toEqual([ 'BrokenRender', 'ErrorBoundary componentDidCatch', 'ErrorBoundary render error', @@ -398,7 +503,6 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.flush(); }).toThrow('Hello'); expect(ops).toEqual(['BrokenRender']); - ops = []; ReactNoop.render(); ReactNoop.flush(); @@ -951,4 +1055,108 @@ describe('ReactIncrementalErrorHandling', () => { }); expect(() => ReactNoop.flush()).toThrow('Error!'); }); + + it('error boundaries do not capture non-errors', () => { + let ops = []; + + class ErrorBoundary extends React.Component { + state = {error: null}; + componentDidCatch(error) { + // Should not be called + ops.push('componentDidCatch'); + this.setState({error}); + } + render() { + if (this.state.error) { + ops.push('ErrorBoundary (catch)'); + return ; + } + ops.push('ErrorBoundary (try)'); + return this.props.children; + } + } + + function Indirection(props) { + ops.push('Indirection'); + return props.children; + } + + const notAnError = {}; + function BadRender() { + ops.push('BadRender'); + throw notAnError; + } + + ReactNoop.render( + + + + + , + ); + + let caught; + try { + ReactNoop.flush(); + } catch (e) { + caught = e; + } + // Rethrow object at the root + expect(caught).toBe(notAnError); + expect(ops).toEqual(['ErrorBoundary (try)', 'Indirection', 'BadRender']); + }); + + it('continues working on siblings of a component that throws', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + componentDidCatch(error) { + ReactNoop.yield('componentDidCatch'); + this.setState({error}); + } + render() { + if (this.state.error) { + ReactNoop.yield('ErrorBoundary (catch)'); + return ; + } + ReactNoop.yield('ErrorBoundary (try)'); + return this.props.children; + } + } + + function ErrorMessage(props) { + ReactNoop.yield('ErrorMessage'); + return ; + } + + function BadRenderSibling(props) { + ReactNoop.yield('BadRenderSibling'); + return null; + } + + function BadRender() { + ReactNoop.yield('throw'); + throw new Error('oops!'); + } + + ReactNoop.render( + + + + + , + ); + + expect(ReactNoop.flush()).toEqual([ + 'ErrorBoundary (try)', + 'throw', + // Continue rendering siblings after BadRender throws + 'BadRenderSibling', + 'BadRenderSibling', + // Recover from the error + 'componentDidCatch', + 'ErrorBoundary (catch)', + 'ErrorMessage', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Caught an error: oops!')]); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js index 15630c1f496..25ff074ca78 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js @@ -179,11 +179,20 @@ describe('ReactIncrementalErrorLogging', () => { let renderAttempts = 0; class ErrorBoundaryComponent extends React.Component { + state = {error: null}; componentDidCatch(error) { handleErrorCalls.push(error); - this.setState({}); // Render again + if (renderAttempts < 10) { + // Render again + } else { + // Stop after 10 tries + this.setState({error}); + } } render() { + if (this.state.error !== null) { + return null; + } return ; } } @@ -208,9 +217,9 @@ describe('ReactIncrementalErrorLogging', () => { ReactNoop.flush(); } catch (error) {} - expect(renderAttempts).toBe(2); - expect(handleErrorCalls.length).toBe(1); - expect(console.error.calls.count()).toBe(2); + expect(renderAttempts).toBe(10); + expect(handleErrorCalls.length).toBe(10); + expect(console.error.calls.count()).toBe(10); if (__DEV__) { expect(console.error.calls.argsFor(0)[0]).toContain( 'The above error occurred in the component:', @@ -223,8 +232,8 @@ describe('ReactIncrementalErrorLogging', () => { 'The above error occurred in the component:', ); expect(console.error.calls.argsFor(1)[0]).toContain( - 'This error was initially handled by the error boundary ErrorBoundaryComponent.\n' + - 'Recreating the tree from scratch failed so React will unmount the tree.', + 'React will try to recreate this component tree from scratch using ' + + 'the error boundary you provided, ErrorBoundaryComponent', ); } }); diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index 753ac38ef07..c8d50898954 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -221,22 +221,15 @@ exports[`ReactDebugFiberPerf recovers from caught errors 1`] = ` // Stop on Baddie and restart from Boundary ⚛ (React Tree Reconciliation) ⚛ Parent [mount] - ⛔ Boundary [mount] Warning: An error was thrown inside this error boundary + ⚛ Boundary [mount] ⚛ Parent [mount] ⚛ Baddie [mount] - ⚛ Boundary [mount] - -⛔ (Committing Changes) Warning: Lifecycle hook scheduled a cascading update - ⚛ (Committing Host Effects: 2 Total) - ⚛ (Calling Lifecycle Methods: 1 Total) - -⚛ (React Tree Reconciliation) - ⚛ Boundary [update] - ⚛ ErrorReport [mount] + ⛔ Boundary [mount] Warning: An error was thrown inside this error boundary + ⚛ ErrorReport [mount] ⚛ (Committing Changes) ⚛ (Committing Host Effects: 2 Total) - ⚛ (Calling Lifecycle Methods: 1 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) " `; @@ -249,8 +242,8 @@ exports[`ReactDebugFiberPerf recovers from fatal errors 1`] = ` ⚛ Baddie [mount] ⚛ (Committing Changes) - ⚛ (Committing Host Effects: 1 Total) - ⚛ (Calling Lifecycle Methods: 1 Total) + ⚛ (Committing Host Effects: 0 Total) + ⚛ (Calling Lifecycle Methods: 0 Total) ⚛ (Waiting for async callback...) diff --git a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js index 40d3d6460f8..357c21dddb1 100644 --- a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js +++ b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js @@ -7,7 +7,7 @@ * @flow */ -import type {CapturedError} from '../ReactFiberScheduler'; +import type {CapturedError} from '../ReactCapturedValue'; // Module provided by RN: import ExceptionsManager from 'ExceptionsManager'; diff --git a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js index b7cd2d59d01..16d9f71c4c9 100644 --- a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js +++ b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js @@ -7,7 +7,7 @@ * @flow */ -import type {CapturedError} from '../ReactFiberScheduler'; +import type {CapturedError} from '../ReactCapturedValue'; import invariant from 'fbjs/lib/invariant'; diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js index 3d461d293b5..5efc96fc6f0 100644 --- a/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.js @@ -460,8 +460,8 @@ describe('ReactTestRenderer', () => { expect(log).toEqual([ 'Boundary render', 'Angry render', - 'Boundary componentDidMount', 'Boundary render', + 'Boundary componentDidMount', ]); });