diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 06f51b0f39741..204c67ba52846 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -2473,4 +2473,175 @@ describe('ReactErrorBoundaries', () => { 'Caught an error: gotta catch em all.', ); }); + + // @gate skipUnmountedBoundaries + it('catches errors thrown in componentWillUnmount', () => { + class LocalErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + Scheduler.unstable_yieldValue( + `ErrorBoundary static getDerivedStateFromError`, + ); + return {error}; + } + render() { + const {children, id, fallbackID} = this.props; + const {error} = this.state; + if (error) { + Scheduler.unstable_yieldValue(`${id} render error`); + return ; + } + Scheduler.unstable_yieldValue(`${id} render success`); + return children || null; + } + } + + class Component extends React.Component { + render() { + const {id} = this.props; + Scheduler.unstable_yieldValue('Component render ' + id); + return id; + } + } + + class LocalBrokenComponentWillUnmount extends React.Component { + componentWillUnmount() { + Scheduler.unstable_yieldValue( + 'BrokenComponentWillUnmount componentWillUnmount', + ); + throw Error('Expected'); + } + + render() { + Scheduler.unstable_yieldValue('BrokenComponentWillUnmount render'); + return 'broken'; + } + } + + const container = document.createElement('div'); + + ReactDOM.render( + + + + + + , + container, + ); + + expect(container.firstChild.textContent).toBe('sibling'); + expect(container.lastChild.textContent).toBe('broken'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'InnerBoundary render success', + 'BrokenComponentWillUnmount render', + ]); + + ReactDOM.render( + + + , + container, + ); + + // React should skip over the unmounting boundary and find the nearest still-mounted boundary. + expect(container.firstChild.textContent).toBe('OuterFallback'); + expect(container.lastChild.textContent).toBe('OuterFallback'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'BrokenComponentWillUnmount componentWillUnmount', + 'ErrorBoundary static getDerivedStateFromError', + 'OuterBoundary render error', + 'Component render OuterFallback', + ]); + }); + + // @gate skipUnmountedBoundaries + it('catches errors thrown while detaching refs', () => { + class LocalErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + Scheduler.unstable_yieldValue( + `ErrorBoundary static getDerivedStateFromError`, + ); + return {error}; + } + render() { + const {children, id, fallbackID} = this.props; + const {error} = this.state; + if (error) { + Scheduler.unstable_yieldValue(`${id} render error`); + return ; + } + Scheduler.unstable_yieldValue(`${id} render success`); + return children || null; + } + } + + class Component extends React.Component { + render() { + const {id} = this.props; + Scheduler.unstable_yieldValue('Component render ' + id); + return id; + } + } + + class LocalBrokenCallbackRef extends React.Component { + _ref = ref => { + Scheduler.unstable_yieldValue('LocalBrokenCallbackRef ref ' + !!ref); + if (ref === null) { + throw Error('Expected'); + } + }; + + render() { + Scheduler.unstable_yieldValue('LocalBrokenCallbackRef render'); + return
ref
; + } + } + + const container = document.createElement('div'); + + ReactDOM.render( + + + + + + , + container, + ); + + expect(container.firstChild.textContent).toBe('sibling'); + expect(container.lastChild.textContent).toBe('ref'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'InnerBoundary render success', + 'LocalBrokenCallbackRef render', + 'LocalBrokenCallbackRef ref true', + ]); + + ReactDOM.render( + + + , + container, + ); + + // React should skip over the unmounting boundary and find the nearest still-mounted boundary. + expect(container.firstChild.textContent).toBe('OuterFallback'); + expect(container.lastChild.textContent).toBe('OuterFallback'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'LocalBrokenCallbackRef ref false', + 'ErrorBoundary static getDerivedStateFromError', + 'OuterBoundary render error', + 'Component render OuterFallback', + ]); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index f086e47cf93b1..727df9a08ffac 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -162,7 +162,11 @@ const callComponentWillUnmountWithTimer = function(current, instance) { }; // Capture errors so they don't interrupt unmounting. -function safelyCallComponentWillUnmount(current, instance) { +function safelyCallComponentWillUnmount( + current: Fiber, + instance: any, + nearestMountedAncestor: Fiber, +) { if (__DEV__) { invokeGuardedCallback( null, @@ -173,18 +177,18 @@ function safelyCallComponentWillUnmount(current, instance) { ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(current, current.return, unmountError); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } else { try { callComponentWillUnmountWithTimer(current, instance); } catch (unmountError) { - captureCommitPhaseError(current, current.return, unmountError); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } } -function safelyDetachRef(current: Fiber) { +function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber) { const ref = current.ref; if (ref !== null) { if (typeof ref === 'function') { @@ -192,13 +196,13 @@ function safelyDetachRef(current: Fiber) { invokeGuardedCallback(null, ref, null, null); if (hasCaughtError()) { const refError = clearCaughtError(); - captureCommitPhaseError(current, current.return, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } else { try { ref(null); } catch (refError) { - captureCommitPhaseError(current, current.return, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } } else { @@ -974,6 +978,7 @@ function commitDetachRef(current: Fiber) { function commitUnmount( finishedRoot: FiberRoot, current: Fiber, + nearestMountedAncestor: Fiber, renderPriorityLevel: ReactPriorityLevel, ): void { onCommitUnmount(current); @@ -1001,10 +1006,10 @@ function commitUnmount( current.mode & ProfileMode ) { startLayoutEffectTimer(); - safelyCallDestroy(current, current.return, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); recordLayoutEffectDuration(current); } else { - safelyCallDestroy(current, current.return, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); } } } @@ -1015,15 +1020,19 @@ function commitUnmount( return; } case ClassComponent: { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); const instance = current.stateNode; if (typeof instance.componentWillUnmount === 'function') { - safelyCallComponentWillUnmount(current, instance); + safelyCallComponentWillUnmount( + current, + instance, + nearestMountedAncestor, + ); } return; } case HostComponent: { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); return; } case HostPortal: { @@ -1031,7 +1040,12 @@ function commitUnmount( // We are also not using this parent because // the portal will get pushed immediately. if (supportsMutation) { - unmountHostComponents(finishedRoot, current, renderPriorityLevel); + unmountHostComponents( + finishedRoot, + current, + nearestMountedAncestor, + renderPriorityLevel, + ); } else if (supportsPersistence) { emptyPortalContainer(current); } @@ -1061,7 +1075,7 @@ function commitUnmount( } case ScopeComponent: { if (enableScopeAPI) { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); } return; } @@ -1071,6 +1085,7 @@ function commitUnmount( function commitNestedUnmounts( finishedRoot: FiberRoot, root: Fiber, + nearestMountedAncestor: Fiber, renderPriorityLevel: ReactPriorityLevel, ): void { // While we're inside a removed host node we don't want to call @@ -1080,7 +1095,12 @@ function commitNestedUnmounts( // we do an inner loop while we're still inside the host node. let node: Fiber = root; while (true) { - commitUnmount(finishedRoot, node, renderPriorityLevel); + commitUnmount( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // Visit children because they may contain more composite or host nodes. // Skip portals because commitUnmount() currently visits them recursively. if ( @@ -1361,9 +1381,10 @@ function insertOrAppendPlacementNode( } function unmountHostComponents( - finishedRoot, - current, - renderPriorityLevel, + finishedRoot: FiberRoot, + current: Fiber, + nearestMountedAncestor: Fiber, + renderPriorityLevel: ReactPriorityLevel, ): void { // We only have the top Fiber that was deleted but we need to recurse down its // children to find all the terminal nodes. @@ -1412,7 +1433,12 @@ function unmountHostComponents( } if (node.tag === HostComponent || node.tag === HostText) { - commitNestedUnmounts(finishedRoot, node, renderPriorityLevel); + commitNestedUnmounts( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // After all the children have unmounted, it is now safe to remove the // node from the tree. if (currentParentIsContainer) { @@ -1429,7 +1455,12 @@ function unmountHostComponents( // Don't visit children because we already visited them. } else if (enableFundamentalAPI && node.tag === FundamentalComponent) { const fundamentalNode = node.stateNode.instance; - commitNestedUnmounts(finishedRoot, node, renderPriorityLevel); + commitNestedUnmounts( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // After all the children have unmounted, it is now safe to remove the // node from the tree. if (currentParentIsContainer) { @@ -1481,7 +1512,12 @@ function unmountHostComponents( continue; } } else { - commitUnmount(finishedRoot, node, renderPriorityLevel); + commitUnmount( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // Visit children because we may find more host components below. if (node.child !== null) { node.child.return = node; @@ -1511,15 +1547,26 @@ function unmountHostComponents( function commitDeletion( finishedRoot: FiberRoot, current: Fiber, + nearestMountedAncestor: Fiber, renderPriorityLevel: ReactPriorityLevel, ): void { if (supportsMutation) { // Recursively delete all host nodes from the parent. // Detach refs and call componentWillUnmount() on the whole subtree. - unmountHostComponents(finishedRoot, current, renderPriorityLevel); + unmountHostComponents( + finishedRoot, + current, + nearestMountedAncestor, + renderPriorityLevel, + ); } else { // Detach refs and call componentWillUnmount() on the whole subtree. - commitNestedUnmounts(finishedRoot, current, renderPriorityLevel); + commitNestedUnmounts( + finishedRoot, + current, + nearestMountedAncestor, + renderPriorityLevel, + ); } const alternate = current.alternate; detachFiberMutation(current); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 8e280e1600510..3265edd9cf584 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -153,7 +153,11 @@ const callComponentWillUnmountWithTimer = function(current, instance) { }; // Capture errors so they don't interrupt unmounting. -function safelyCallComponentWillUnmount(current, instance) { +function safelyCallComponentWillUnmount( + current: Fiber, + instance: any, + nearestMountedAncestor: Fiber | null, +) { if (__DEV__) { invokeGuardedCallback( null, @@ -164,18 +168,18 @@ function safelyCallComponentWillUnmount(current, instance) { ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(current, unmountError); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } else { try { callComponentWillUnmountWithTimer(current, instance); } catch (unmountError) { - captureCommitPhaseError(current, unmountError); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } } -function safelyDetachRef(current: Fiber) { +function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; if (ref !== null) { if (typeof ref === 'function') { @@ -183,13 +187,13 @@ function safelyDetachRef(current: Fiber) { invokeGuardedCallback(null, ref, null, null); if (hasCaughtError()) { const refError = clearCaughtError(); - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } else { try { ref(null); } catch (refError) { - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } } else { @@ -198,18 +202,22 @@ function safelyDetachRef(current: Fiber) { } } -function safelyCallDestroy(current, destroy) { +function safelyCallDestroy( + current: Fiber, + nearestMountedAncestor: Fiber | null, + destroy: () => void, +) { if (__DEV__) { invokeGuardedCallback(null, destroy, null); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(current, error); + captureCommitPhaseError(current, nearestMountedAncestor, error); } } else { try { destroy(); } catch (error) { - captureCommitPhaseError(current, error); + captureCommitPhaseError(current, nearestMountedAncestor, error); } } } @@ -866,6 +874,7 @@ function commitDetachRef(current: Fiber) { function commitUnmount( finishedRoot: FiberRoot, current: Fiber, + nearestMountedAncestor: Fiber | null, renderPriorityLevel: ReactPriorityLevel, ): void { onCommitUnmount(current); @@ -895,10 +904,10 @@ function commitUnmount( current.mode & ProfileMode ) { startLayoutEffectTimer(); - safelyCallDestroy(current, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); recordLayoutEffectDuration(current); } else { - safelyCallDestroy(current, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); } } } @@ -909,15 +918,19 @@ function commitUnmount( return; } case ClassComponent: { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); const instance = current.stateNode; if (typeof instance.componentWillUnmount === 'function') { - safelyCallComponentWillUnmount(current, instance); + safelyCallComponentWillUnmount( + current, + instance, + nearestMountedAncestor, + ); } return; } case HostComponent: { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); return; } case HostPortal: { @@ -925,7 +938,12 @@ function commitUnmount( // We are also not using this parent because // the portal will get pushed immediately. if (supportsMutation) { - unmountHostComponents(finishedRoot, current, renderPriorityLevel); + unmountHostComponents( + finishedRoot, + current, + nearestMountedAncestor, + renderPriorityLevel, + ); } else if (supportsPersistence) { emptyPortalContainer(current); } @@ -955,7 +973,7 @@ function commitUnmount( } case ScopeComponent: { if (enableScopeAPI) { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); } return; } @@ -965,6 +983,7 @@ function commitUnmount( function commitNestedUnmounts( finishedRoot: FiberRoot, root: Fiber, + nearestMountedAncestor: Fiber | null, renderPriorityLevel: ReactPriorityLevel, ): void { // While we're inside a removed host node we don't want to call @@ -974,7 +993,12 @@ function commitNestedUnmounts( // we do an inner loop while we're still inside the host node. let node: Fiber = root; while (true) { - commitUnmount(finishedRoot, node, renderPriorityLevel); + commitUnmount( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // Visit children because they may contain more composite or host nodes. // Skip portals because commitUnmount() currently visits them recursively. if ( @@ -1263,9 +1287,10 @@ function insertOrAppendPlacementNode( } function unmountHostComponents( - finishedRoot, - current, - renderPriorityLevel, + finishedRoot: FiberRoot, + current: Fiber, + nearestMountedAncestor: Fiber | null, + renderPriorityLevel: ReactPriorityLevel, ): void { // We only have the top Fiber that was deleted but we need to recurse down its // children to find all the terminal nodes. @@ -1314,7 +1339,12 @@ function unmountHostComponents( } if (node.tag === HostComponent || node.tag === HostText) { - commitNestedUnmounts(finishedRoot, node, renderPriorityLevel); + commitNestedUnmounts( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // After all the children have unmounted, it is now safe to remove the // node from the tree. if (currentParentIsContainer) { @@ -1331,7 +1361,12 @@ function unmountHostComponents( // Don't visit children because we already visited them. } else if (enableFundamentalAPI && node.tag === FundamentalComponent) { const fundamentalNode = node.stateNode.instance; - commitNestedUnmounts(finishedRoot, node, renderPriorityLevel); + commitNestedUnmounts( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // After all the children have unmounted, it is now safe to remove the // node from the tree. if (currentParentIsContainer) { @@ -1383,7 +1418,12 @@ function unmountHostComponents( continue; } } else { - commitUnmount(finishedRoot, node, renderPriorityLevel); + commitUnmount( + finishedRoot, + node, + nearestMountedAncestor, + renderPriorityLevel, + ); // Visit children because we may find more host components below. if (node.child !== null) { node.child.return = node; @@ -1413,15 +1453,26 @@ function unmountHostComponents( function commitDeletion( finishedRoot: FiberRoot, current: Fiber, + nearestMountedAncestor: Fiber | null, renderPriorityLevel: ReactPriorityLevel, ): void { if (supportsMutation) { // Recursively delete all host nodes from the parent. // Detach refs and call componentWillUnmount() on the whole subtree. - unmountHostComponents(finishedRoot, current, renderPriorityLevel); + unmountHostComponents( + finishedRoot, + current, + nearestMountedAncestor, + renderPriorityLevel, + ); } else { // Detach refs and call componentWillUnmount() on the whole subtree. - commitNestedUnmounts(finishedRoot, current, renderPriorityLevel); + commitNestedUnmounts( + finishedRoot, + current, + nearestMountedAncestor, + renderPriorityLevel, + ); } const alternate = current.alternate; detachFiberMutation(current); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 87da91d58c5f4..c7d1faef4cfbf 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -30,6 +30,7 @@ import { enableDebugTracing, enableSchedulingProfiler, enableScopeAPI, + skipUnmountedBoundaries, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -2462,13 +2463,18 @@ function commitBeforeMutationEffectsDeletions(deletions: Array) { function commitMutationEffects( firstChild: Fiber, root: FiberRoot, - renderPriorityLevel, + renderPriorityLevel: ReactPriorityLevel, ) { let fiber = firstChild; while (fiber !== null) { const deletions = fiber.deletions; if (deletions !== null) { - commitMutationEffectsDeletions(deletions, root, renderPriorityLevel); + commitMutationEffectsDeletions( + deletions, + fiber, + root, + renderPriorityLevel, + ); } if (fiber.child !== null) { @@ -2577,6 +2583,7 @@ function commitMutationEffectsImpl( function commitMutationEffectsDeletions( deletions: Array, + nearestMountedAncestor: Fiber, root: FiberRoot, renderPriorityLevel, ) { @@ -2589,17 +2596,23 @@ function commitMutationEffectsDeletions( null, root, childToDelete, + nearestMountedAncestor, renderPriorityLevel, ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(childToDelete, childToDelete.return, error); + captureCommitPhaseError(childToDelete, nearestMountedAncestor, error); } } else { try { - commitDeletion(root, childToDelete, renderPriorityLevel); + commitDeletion( + root, + childToDelete, + nearestMountedAncestor, + renderPriorityLevel, + ); } catch (error) { - captureCommitPhaseError(childToDelete, childToDelete.return, error); + captureCommitPhaseError(childToDelete, nearestMountedAncestor, error); } } } @@ -2938,7 +2951,13 @@ export function captureCommitPhaseError( return; } - let fiber = nearestMountedAncestor; + let fiber = null; + if (skipUnmountedBoundaries) { + fiber = nearestMountedAncestor; + } else { + fiber = sourceFiber.return; + } + while (fiber !== null) { if (fiber.tag === HostRoot) { captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 84188fea37427..ff3fd6a9d0e41 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -30,6 +30,7 @@ import { enableDebugTracing, enableSchedulingProfiler, enableScopeAPI, + skipUnmountedBoundaries, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -2084,7 +2085,7 @@ function commitRootImpl(root, renderPriorityLevel) { if (hasCaughtError()) { invariant(nextEffect !== null, 'Should be working on an effect.'); const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); + captureCommitPhaseError(nextEffect, nextEffect.return, error); nextEffect = nextEffect.nextEffect; } } else { @@ -2092,7 +2093,7 @@ function commitRootImpl(root, renderPriorityLevel) { commitBeforeMutationEffects(); } catch (error) { invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); + captureCommitPhaseError(nextEffect, nextEffect.return, error); nextEffect = nextEffect.nextEffect; } } @@ -2121,7 +2122,7 @@ function commitRootImpl(root, renderPriorityLevel) { if (hasCaughtError()) { invariant(nextEffect !== null, 'Should be working on an effect.'); const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); + captureCommitPhaseError(nextEffect, nextEffect.return, error); nextEffect = nextEffect.nextEffect; } } else { @@ -2129,7 +2130,7 @@ function commitRootImpl(root, renderPriorityLevel) { commitMutationEffects(root, renderPriorityLevel); } catch (error) { invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); + captureCommitPhaseError(nextEffect, nextEffect.return, error); nextEffect = nextEffect.nextEffect; } } @@ -2156,7 +2157,7 @@ function commitRootImpl(root, renderPriorityLevel) { if (hasCaughtError()) { invariant(nextEffect !== null, 'Should be working on an effect.'); const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); + captureCommitPhaseError(nextEffect, nextEffect.return, error); nextEffect = nextEffect.nextEffect; } } else { @@ -2164,7 +2165,7 @@ function commitRootImpl(root, renderPriorityLevel) { commitLayoutEffects(root, lanes); } catch (error) { invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); + captureCommitPhaseError(nextEffect, nextEffect.return, error); nextEffect = nextEffect.nextEffect; } } @@ -2365,7 +2366,10 @@ function commitBeforeMutationEffects() { } } -function commitMutationEffects(root: FiberRoot, renderPriorityLevel) { +function commitMutationEffects( + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { // TODO: Should probably move the bulk of this function to commitWork. while (nextEffect !== null) { setCurrentDebugFiberInDEV(nextEffect); @@ -2436,7 +2440,12 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) { break; } case Deletion: { - commitDeletion(root, nextEffect, renderPriorityLevel); + commitDeletion( + root, + nextEffect, + nextEffect.return, + renderPriorityLevel, + ); break; } } @@ -2647,7 +2656,7 @@ function flushPassiveEffectsImpl() { if (hasCaughtError()) { invariant(fiber !== null, 'Should be working on an effect.'); const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { @@ -2668,7 +2677,7 @@ function flushPassiveEffectsImpl() { } } catch (error) { invariant(fiber !== null, 'Should be working on an effect.'); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2695,7 +2704,7 @@ function flushPassiveEffectsImpl() { if (hasCaughtError()) { invariant(fiber !== null, 'Should be working on an effect.'); const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { @@ -2717,7 +2726,7 @@ function flushPassiveEffectsImpl() { } } catch (error) { invariant(fiber !== null, 'Should be working on an effect.'); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2816,7 +2825,11 @@ function captureCommitPhaseErrorOnRoot( } } -export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) { +export function captureCommitPhaseError( + sourceFiber: Fiber, + nearestMountedAncestor: Fiber | null, + error: mixed, +) { if (sourceFiber.tag === HostRoot) { // Error was thrown at the root. There is no parent, so the root // itself should capture it. @@ -2824,7 +2837,13 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) { return; } - let fiber = sourceFiber.return; + let fiber = null; + if (skipUnmountedBoundaries) { + fiber = nearestMountedAncestor; + } else { + fiber = sourceFiber.return; + } + while (fiber !== null) { if (fiber.tag === HostRoot) { captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 7ff61126f648a..7fa56978e74a2 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2797,6 +2797,85 @@ describe('ReactHooksWithNoopRenderer', () => { 'Mount normal [current: 1]', ]); }); + + // @gate skipUnmountedBoundaries + it('catches errors thrown in useLayoutEffect', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + Scheduler.unstable_yieldValue( + `ErrorBoundary static getDerivedStateFromError`, + ); + return {error}; + } + render() { + const {children, id, fallbackID} = this.props; + const {error} = this.state; + if (error) { + Scheduler.unstable_yieldValue(`${id} render error`); + return ; + } + Scheduler.unstable_yieldValue(`${id} render success`); + return children || null; + } + } + + function Component({id}) { + Scheduler.unstable_yieldValue('Component render ' + id); + return ; + } + + function BrokenLayoutEffectDestroy() { + useLayoutEffect(() => { + return () => { + Scheduler.unstable_yieldValue( + 'BrokenLayoutEffectDestroy useLayoutEffect destroy', + ); + throw Error('Expected'); + }; + }, []); + + Scheduler.unstable_yieldValue('BrokenLayoutEffectDestroy render'); + return ; + } + + ReactNoop.render( + + + + + + , + ); + + expect(Scheduler).toFlushAndYield([ + 'OuterBoundary render success', + 'Component render sibling', + 'InnerBoundary render success', + 'BrokenLayoutEffectDestroy render', + ]); + expect(ReactNoop.getChildren()).toEqual([ + span('sibling'), + span('broken'), + ]); + + ReactNoop.render( + + + , + ); + + // React should skip over the unmounting boundary and find the nearest still-mounted boundary. + expect(Scheduler).toFlushAndYield([ + 'OuterBoundary render success', + 'Component render sibling', + 'BrokenLayoutEffectDestroy useLayoutEffect destroy', + 'ErrorBoundary static getDerivedStateFromError', + 'OuterBoundary render error', + 'Component render OuterFallback', + ]); + expect(ReactNoop.getChildren()).toEqual([span('OuterFallback')]); + }); }); describe('useCallback', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 503dc98f0de41..06668b4eb3896 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -961,6 +961,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(Scheduler).toFlushAndYield(['Foo']); }); + // @gate skipUnmountedBoundaries it('should not attempt to recover an unmounting error boundary', () => { class Parent extends React.Component { componentWillUnmount() { @@ -992,12 +993,17 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); - ReactNoop.render(null); - expect(Scheduler).toFlushAndYield([ - // Parent unmounts before the error is thrown. - 'Parent componentWillUnmount', - 'ThrowsOnUnmount componentWillUnmount', - ]); + + // Because the error boundary is also unmounting, + // an error in ThrowsOnUnmount should be rethrown. + expect(() => { + ReactNoop.render(null); + expect(Scheduler).toFlushAndYield([ + 'Parent componentWillUnmount', + 'ThrowsOnUnmount componentWillUnmount', + ]); + }).toThrow('unmount error'); + ReactNoop.render(); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 634c19b14ea52..2ae1805af95c1 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -92,6 +92,12 @@ export const enableComponentStackLocations = true; export const enableNewReconciler = false; +// Errors that are thrown while unmounting (or after in the case of passive effects) +// should bypass any error boundaries that are also unmounting (or have unmounted) +// and be handled by the nearest still-mounted boundary. +// If there are no still-mounted boundaries, the errors should be rethrown. +export const skipUnmountedBoundaries = false; + // -------------------------- // Future APIs to be deprecated // -------------------------- diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 664a11a43b981..6b61818c6d044 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -43,6 +43,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 9632b8b7da08b..c8598c23cd924 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6f348ed149b22..47a729c1d0a09 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index ea8f7d4e0a97e..4af72e9d901a7 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 3e5046f0fccd8..7c25d50d07de9 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 8d4fe939dc09b..5e530afbd9db7 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index d3796937a3028..e11f9f9d13381 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; +export const skipUnmountedBoundaries = __EXPERIMENTAL__; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 9b23645a3d87e..c7df70d192989 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -18,6 +18,7 @@ export const disableInputAttributeSyncing = __VARIANT__; export const enableFilterEmptyStringAttributesDOM = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__; export const decoupleUpdatePriorityFromScheduler = __VARIANT__; +export const skipUnmountedBoundaries = __VARIANT__; // Enable this flag to help with concurrent mode debugging. // It logs information to the console about React scheduling, rendering, and commit phases. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 1c637762fe6b9..35f5767413fc3 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -26,6 +26,7 @@ export const { deferRenderPhaseUpdateToNextBatch, decoupleUpdatePriorityFromScheduler, enableDebugTracing, + skipUnmountedBoundaries, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.