From c74790fd1ed2b7a654e87cc20195758bf892d849 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 17 Aug 2020 11:18:37 -0400 Subject: [PATCH] Always skip unmounted/unmounting error boundaries The behavior of error boundaries for passive effects that throw during cleanup was recently changed so that React ignores boundaries which are also unmounting in favor of still-mounted boundaries. This commit implements that same behavior for layout effects (useLayoutEffect and componentWillUnmount). --- .../ReactErrorBoundaries-test.internal.js | 84 +++++++++++++++++ .../src/ReactFiberCommitWork.new.js | 79 ++++++++++++---- .../src/ReactFiberCommitWork.old.js | 93 ++++++++++++++----- .../src/ReactFiberWorkLoop.new.js | 22 ++++- .../src/ReactFiberWorkLoop.old.js | 40 +++++--- .../ReactHooksWithNoopRenderer-test.js | 78 ++++++++++++++++ ...tIncrementalErrorHandling-test.internal.js | 17 ++-- 7 files changed, 351 insertions(+), 62 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 06f51b0f39741..bf8fd72b730cc 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -2473,4 +2473,88 @@ describe('ReactErrorBoundaries', () => { 'Caught an error: gotta catch em all.', ); }); + + 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', + ]); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index f086e47cf93b1..c451585a0745f 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,13 +177,13 @@ 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); } } } @@ -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); } } } @@ -1018,7 +1023,11 @@ function commitUnmount( safelyDetachRef(current); const instance = current.stateNode; if (typeof instance.componentWillUnmount === 'function') { - safelyCallComponentWillUnmount(current, instance); + safelyCallComponentWillUnmount( + current, + instance, + nearestMountedAncestor, + ); } return; } @@ -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); } @@ -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..81b226670f179 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,13 +168,13 @@ 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); } } } @@ -183,13 +187,13 @@ function safelyDetachRef(current: Fiber) { invokeGuardedCallback(null, ref, null, null); if (hasCaughtError()) { const refError = clearCaughtError(); - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, current.return, refError); } } else { try { ref(null); } catch (refError) { - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, current.return, 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); } } } @@ -912,7 +921,11 @@ function commitUnmount( safelyDetachRef(current); const instance = current.stateNode; if (typeof instance.componentWillUnmount === 'function') { - safelyCallComponentWillUnmount(current, instance); + safelyCallComponentWillUnmount( + current, + instance, + nearestMountedAncestor, + ); } return; } @@ -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); } @@ -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..578ffd427c794 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -2462,13 +2462,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 +2582,7 @@ function commitMutationEffectsImpl( function commitMutationEffectsDeletions( deletions: Array, + nearestMountedAncestor: Fiber, root: FiberRoot, renderPriorityLevel, ) { @@ -2589,17 +2595,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); } } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 84188fea37427..d40a164b94abc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -2084,7 +2084,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 +2092,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 +2121,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 +2129,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 +2156,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 +2164,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 +2365,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 +2439,12 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) { break; } case Deletion: { - commitDeletion(root, nextEffect, renderPriorityLevel); + commitDeletion( + root, + nextEffect, + nextEffect.return, + renderPriorityLevel, + ); break; } } @@ -2647,7 +2655,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 +2676,7 @@ function flushPassiveEffectsImpl() { } } catch (error) { invariant(fiber !== null, 'Should be working on an effect.'); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2695,7 +2703,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 +2725,7 @@ function flushPassiveEffectsImpl() { } } catch (error) { invariant(fiber !== null, 'Should be working on an effect.'); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2816,7 +2824,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 +2836,7 @@ export function captureCommitPhaseError(sourceFiber: Fiber, error: mixed) { return; } - let fiber = sourceFiber.return; + let fiber = nearestMountedAncestor; 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..75e52dd951b79 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2797,6 +2797,84 @@ describe('ReactHooksWithNoopRenderer', () => { 'Mount normal [current: 1]', ]); }); + + 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..f41c9f661a14b 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -992,12 +992,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(); });