diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 67bbbea71ec..c78722ebb6f 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,9 +87,6 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate -src/renderers/dom/__tests__/ReactDOMProduction-test.js -* should throw with an error code in production - src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index b42a892af2c..71f078f4096 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,6 +438,7 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods +* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 10ef91b8e44..7a902e20977 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -12,7 +12,6 @@ 'use strict'; -import type { TrappedError } from 'ReactFiberScheduler'; import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; @@ -34,7 +33,7 @@ var { module.exports = function( config : HostConfig, - trapError : (boundary : Fiber, error: Error) => TrappedError + trapError : (fiber : Fiber, error: Error, isUnmounting : boolean) => void ) { const updateContainer = config.updateContainer; @@ -158,10 +157,7 @@ module.exports = function( } } - function commitNestedUnmounts(root : Fiber): Array | null { - // Since errors are rare, we allocate this array on demand. - let trappedErrors = null; - + function commitNestedUnmounts(root : Fiber): void { // While we're inside a removed host node we don't want to call // removeChild on the inner nodes because they're removed by the top // call anyway. We also want to call componentWillUnmount on all @@ -169,58 +165,39 @@ module.exports = function( // we do an inner loop while we're still inside the host node. let node : Fiber = root; while (true) { - const error = commitUnmount(node); - if (error) { - trappedErrors = trappedErrors || []; - trappedErrors.push(error); - } + commitUnmount(node); if (node.child) { // TODO: Coroutines need to visit the stateNode. node = node.child; continue; } if (node === root) { - return trappedErrors; + return; } while (!node.sibling) { if (!node.return || node.return === root) { - return trappedErrors; + return; } node = node.return; } node = node.sibling; } - return trappedErrors; } - function unmountHostComponents(parent, current): Array | null { - // Since errors are rare, we allocate this array on demand. - let trappedErrors = null; - + function unmountHostComponents(parent, current): void { // We only have the top Fiber that was inserted but we need recurse down its // children to find all the terminal nodes. let node : Fiber = current; while (true) { if (node.tag === HostComponent || node.tag === HostText) { - const errors = commitNestedUnmounts(node); - if (errors) { - if (!trappedErrors) { - trappedErrors = errors; - } else { - trappedErrors.push.apply(trappedErrors, errors); - } - } + commitNestedUnmounts(node); // After all the children have unmounted, it is now safe to remove the // node from the tree. if (parent) { removeChild(parent, node.stateNode); } } else { - const error = commitUnmount(node); - if (error) { - trappedErrors = trappedErrors || []; - trappedErrors.push(error); - } + commitUnmount(node); if (node.child) { // TODO: Coroutines need to visit the stateNode. node = node.child; @@ -228,24 +205,23 @@ module.exports = function( } } if (node === current) { - return trappedErrors; + return; } while (!node.sibling) { if (!node.return || node.return === current) { - return trappedErrors; + return; } node = node.return; } node = node.sibling; } - return trappedErrors; } - function commitDeletion(current : Fiber) : Array | null { + function commitDeletion(current : Fiber) : void { // Recursively delete all host nodes from the parent. const parent = getHostParent(current); // Detach refs and call componentWillUnmount() on the whole subtree. - const trappedErrors = unmountHostComponents(parent, current); + unmountHostComponents(parent, current); // Cut off the return pointers to disconnect it from the tree. Ideally, we // should clear the child pointer of the parent alternate to let this @@ -258,11 +234,9 @@ module.exports = function( current.alternate.child = null; current.alternate.return = null; } - - return trappedErrors; } - function commitUnmount(current : Fiber) : TrappedError | null { + function commitUnmount(current : Fiber) : void { switch (current.tag) { case ClassComponent: { detachRef(current); @@ -270,17 +244,14 @@ module.exports = function( if (typeof instance.componentWillUnmount === 'function') { const error = tryCallComponentWillUnmount(instance); if (error) { - return trapError(current, error); + trapError(current, error, true); } } - return null; + return; } case HostComponent: { detachRef(current); - return null; - } - default: { - return null; + return; } } } @@ -325,7 +296,7 @@ module.exports = function( } } - function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : TrappedError | null { + function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : void { switch (finishedWork.tag) { case ClassComponent: { const instance = finishedWork.stateNode; @@ -355,9 +326,9 @@ module.exports = function( } } if (error) { - return trapError(finishedWork, error); + trapError(finishedWork, error, false); } - return null; + return; } case HostContainer: { const rootFiber = finishedWork.stateNode; @@ -366,15 +337,16 @@ module.exports = function( rootFiber.callbackList = null; callCallbacks(callbackList, rootFiber.current.child.stateNode); } + return; } case HostComponent: { const instance : I = finishedWork.stateNode; attachRef(current, finishedWork, instance); - return null; + return; } case HostText: { // We have no life-cycles associated with text. - return null; + return; } default: throw new Error('This unit of work tag should not have side-effects.'); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index dafb2652fba..f8f97848892 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -58,7 +58,7 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; -export type TrappedError = { +type TrappedError = { boundary: Fiber | null, error: any, }; @@ -93,10 +93,9 @@ module.exports = function(config : HostConfig) { let isAnimationCallbackScheduled : boolean = false; let isDeferredCallbackScheduled : boolean = false; + // Caught errors and error boundaries that are currently handling them + let activeErrorBoundaries : Set | null = null; let nextTrappedErrors : Array | null = null; - let isHandlingErrors : boolean = false; - // Boundaries that have been acknowledged - let knownBoundaries : Set = new Set(); function scheduleAnimationCallback(callback) { if (!isAnimationCallbackScheduled) { @@ -159,13 +158,6 @@ module.exports = function(config : HostConfig) { function commitAllWork(finishedWork : Fiber, ignoreUnmountingErrors : boolean) { // Commit all the side-effects within a tree. - - // Commit phase is meant to be atomic and non-interruptible. - // Any errors raised in it should be handled after it is over - // so that we don't end up in an inconsistent state due to user code. - // We'll keep track of all caught errors and handle them later. - let allTrappedErrors = null; - // First, we'll perform all the host insertions, updates, deletions and // ref unmounts. let effectfulFiber = finishedWork.firstEffect; @@ -201,18 +193,11 @@ module.exports = function(config : HostConfig) { case DeletionAndCallback: // Deletion might cause an error in componentWillUnmount(). // We will continue nevertheless and handle those later on. - const trappedErrors = commitDeletion(effectfulFiber); - // There is a special case where we completely ignore errors. + // Note: There is a special case where we completely ignore errors. // It happens when we already caught an error earlier, and the update // is caused by an error boundary trying to render an error message. // In this case, we want to blow away the tree without catching errors. - if (trappedErrors && !ignoreUnmountingErrors) { - if (!allTrappedErrors) { - allTrappedErrors = trappedErrors; - } else { - allTrappedErrors.push.apply(allTrappedErrors, trappedErrors); - } - } + commitDeletion(effectfulFiber, ignoreUnmountingErrors); break; } @@ -226,11 +211,7 @@ module.exports = function(config : HostConfig) { while (effectfulFiber) { if (effectfulFiber.effectTag & (Update | Callback)) { const current = effectfulFiber.alternate; - const trappedError = commitLifeCycles(current, effectfulFiber); - if (trappedError) { - allTrappedErrors = allTrappedErrors || []; - allTrappedErrors.push(trappedError); - } + commitLifeCycles(current, effectfulFiber); } const next = effectfulFiber.nextEffect; // Ensure that we clean these up so that we don't accidentally keep them. @@ -248,19 +229,7 @@ module.exports = function(config : HostConfig) { if (finishedWork.effectTag !== NoEffect) { const current = finishedWork.alternate; commitWork(current, finishedWork); - const trappedError = commitLifeCycles(current, finishedWork); - if (trappedError) { - allTrappedErrors = allTrappedErrors || []; - allTrappedErrors.push(trappedError); - } - } - - if (allTrappedErrors) { - if (nextTrappedErrors) { - nextTrappedErrors.push.apply(nextTrappedErrors, allTrappedErrors); - } else { - nextTrappedErrors = allTrappedErrors; - } + commitLifeCycles(current, finishedWork); } performTaskWork(); @@ -519,11 +488,10 @@ module.exports = function(config : HostConfig) { break; } } catch (error) { - const failedUnitOfWork = nextUnitOfWork; - // Clean-up ReactCurrentOwner.current = null; + const failedUnitOfWork = nextUnitOfWork; // Reset because it points to the error boundary: nextUnitOfWork = null; if (!failedUnitOfWork) { @@ -531,12 +499,7 @@ module.exports = function(config : HostConfig) { // should always be set while work is being performed. throw error; } - const trappedError = trapError(failedUnitOfWork, error); - if (nextTrappedErrors) { - nextTrappedErrors.push(trappedError); - } else { - nextTrappedErrors = [trappedError]; - } + trapError(failedUnitOfWork, error, false); } // If there were any errors, handle them now @@ -546,21 +509,27 @@ module.exports = function(config : HostConfig) { } function handleErrors() : void { - // Prevent recursion - if (isHandlingErrors) { + // Prevent recursion. + // TODO: remove the codepath that causes this recursion in the first place. + if (activeErrorBoundaries) { return; } - isHandlingErrors = true; + // Start tracking active boundaries. + activeErrorBoundaries = new Set(); + + // If we find unhandled errors, we'll only rethrow the first one. let firstUncaughtError = null; // All work created by error boundaries should have Task priority - // so that it finishes before this function exits + // so that it finishes before this function exits. const previousPriorityContext = priorityContext; priorityContext = TaskPriority; - // Keep looping until there are no more trapped errors - while (true) { + // Keep looping until there are no more trapped errors, or until we find + // an unhandled error. + while (nextTrappedErrors && nextTrappedErrors.length && !firstUncaughtError) { + // First, find all error boundaries and notify them about errors. while (nextTrappedErrors && nextTrappedErrors.length) { const trappedError = nextTrappedErrors.shift(); const boundary = trappedError.boundary; @@ -570,35 +539,32 @@ module.exports = function(config : HostConfig) { continue; } // Don't visit boundaries twice. - if (knownBoundaries.has(boundary)) { + if (activeErrorBoundaries.has(boundary)) { continue; } try { - knownBoundaries.add(boundary); - + // This error boundary is now active. + // We will let it handle the error and retry rendering. + // If it fails again, the next error will be propagated to the parent + // boundary or rethrown. + activeErrorBoundaries.add(boundary); // Give error boundary a chance to update its state. // Updates will be scheduled with Task priority. - acknowledgeErrorInBoundary(boundary, error); + const instance = boundary.stateNode; + instance.unstable_handleError(error); // Schedule an update, in case the boundary didn't call setState - // on itself + // on itself. scheduleUpdate(boundary); } catch (nextError) { - // If an error is thrown, propagate the error to the next boundary - const te = trapError(boundary, nextError); - if (te.boundary) { - // If a boundary is found, push trapped error onto array - nextTrappedErrors.push(te); - } else { - // Otherwise, we'll need to throw - firstUncaughtError = firstUncaughtError || te.error; - } + // If an error is thrown, propagate the error to the parent boundary. + trapError(boundary, nextError, false); } } - - // Now that we attempt to flush any work that was scheduled by the boundaries - // If this creates errors, they will be pushed to nextTrappedErrors and the loop will continue + // Now that we attempt to flush any work that was scheduled by the boundaries. + // If this creates errors, they will be pushed to nextTrappedErrors and + // the outer loop will continue. try { performTaskWorkUnsafe(true); } catch (error) { @@ -610,26 +576,13 @@ module.exports = function(config : HostConfig) { // should always be set while work is being performed. throw error; } - const trappedError = trapError(failedUnitOfWork, error); - if (nextTrappedErrors) { - nextTrappedErrors.push(trappedError); - } else { - nextTrappedErrors = [trappedError]; - } - } - - - if (!nextTrappedErrors || - !nextTrappedErrors.length || - firstUncaughtError) { - break; + trapError(failedUnitOfWork, error, false); } } nextTrappedErrors = null; - knownBoundaries.clear(); + activeErrorBoundaries = null; priorityContext = previousPriorityContext; - isHandlingErrors = false; if (firstUncaughtError) { // We need to make sure any future root can get scheduled despite these errors. @@ -646,9 +599,15 @@ module.exports = function(config : HostConfig) { while (maybeErrorBoundary) { if (maybeErrorBoundary.tag === ClassComponent) { const instance = maybeErrorBoundary.stateNode; - if (typeof instance.unstable_handleError === 'function' && - !knownBoundaries.has(maybeErrorBoundary)) { - return maybeErrorBoundary; + const isErrorBoundary = typeof instance.unstable_handleError === 'function'; + if (isErrorBoundary) { + const isHandlingAnotherError = ( + activeErrorBoundaries !== null && + activeErrorBoundaries.has(maybeErrorBoundary) + ); + if (!isHandlingAnotherError) { + return maybeErrorBoundary; + } } } maybeErrorBoundary = maybeErrorBoundary.return; @@ -656,16 +615,20 @@ module.exports = function(config : HostConfig) { return null; } - function trapError(fiber : Fiber, error : any) : TrappedError { - return { + function trapError(fiber : Fiber, error : any, isUnmounting : boolean) : void { + if (isUnmounting && activeErrorBoundaries) { + // Ignore errors caused by unmounting during error handling. + // This lets error boundaries safely tear down already failed trees. + return; + } + + if (!nextTrappedErrors) { + nextTrappedErrors = []; + } + nextTrappedErrors.push({ boundary: findClosestErrorBoundary(fiber), error, - }; - } - - function acknowledgeErrorInBoundary(boundary : Fiber, error : any) { - const instance = boundary.stateNode; - instance.unstable_handleError(error); + }); } function scheduleWork(root : FiberRoot) {