diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 50eae32123b71..02e4833685528 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -42,6 +42,7 @@ describe('ReactErrorBoundaries', () => { PropTypes = require('prop-types'); ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; + ReactFeatureFlags.skipUnmountedBoundaries = true; ReactDOM = require('react-dom'); React = require('react'); act = require('react-dom/test-utils').unstable_concurrentAct; @@ -2473,4 +2474,173 @@ 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', + ]); + }); + + 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 2d50f255ee21f..29fbd87720128 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -177,7 +177,11 @@ const callComponentWillUnmountWithTimer = function(current, instance) { }; // Capture errors so they don't interrupt unmounting. -function safelyCallComponentWillUnmount(current: Fiber, instance: any) { +function safelyCallComponentWillUnmount( + current: Fiber, + nearestMountedAncestor: Fiber | null, + instance: any, +) { if (__DEV__) { invokeGuardedCallback( null, @@ -188,18 +192,18 @@ function safelyCallComponentWillUnmount(current: Fiber, instance: any) { ); 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') { @@ -218,7 +222,7 @@ function safelyDetachRef(current: Fiber) { if (hasCaughtError()) { const refError = clearCaughtError(); - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } else { try { @@ -237,7 +241,7 @@ function safelyDetachRef(current: Fiber) { ref(null); } } catch (refError) { - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } } else { @@ -246,18 +250,22 @@ function safelyDetachRef(current: Fiber) { } } -function safelyCallDestroy(current: Fiber, destroy: () => void) { +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); } } } @@ -321,14 +329,14 @@ function commitBeforeMutationEffects_complete() { ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitBeforeMutationEffectsOnFiber(fiber); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } @@ -462,7 +470,11 @@ function commitBeforeMutationEffectsDeletion(deletion: Fiber) { } } -function commitHookEffectListUnmount(flags: HookFlags, finishedWork: Fiber) { +function commitHookEffectListUnmount( + flags: HookFlags, + finishedWork: Fiber, + nearestMountedAncestor: Fiber | null, +) { const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any); const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; if (lastEffect !== null) { @@ -474,7 +486,7 @@ function commitHookEffectListUnmount(flags: HookFlags, finishedWork: Fiber) { const destroy = effect.destroy; effect.destroy = undefined; if (destroy !== undefined) { - safelyCallDestroy(finishedWork, destroy); + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); } } effect = effect.next; @@ -1049,6 +1061,7 @@ function commitDetachRef(current: Fiber) { function commitUnmount( finishedRoot: FiberRoot, current: Fiber, + nearestMountedAncestor: Fiber, renderPriorityLevel: ReactPriorityLevel, ): void { onCommitUnmount(current); @@ -1075,10 +1088,10 @@ function commitUnmount( current.mode & ProfileMode ) { startLayoutEffectTimer(); - safelyCallDestroy(current, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); recordLayoutEffectDuration(current); } else { - safelyCallDestroy(current, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); } } } @@ -1089,15 +1102,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, + nearestMountedAncestor, + instance, + ); } return; } case HostComponent: { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); return; } case HostPortal: { @@ -1105,7 +1122,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); } @@ -1135,7 +1157,7 @@ function commitUnmount( } case ScopeComponent: { if (enableScopeAPI) { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); } return; } @@ -1145,6 +1167,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 @@ -1154,7 +1177,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 ( @@ -1455,6 +1483,7 @@ function insertOrAppendPlacementNode( function unmountHostComponents( 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 @@ -1504,7 +1533,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) { @@ -1521,7 +1555,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) { @@ -1573,7 +1612,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; @@ -1603,15 +1647,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); @@ -1642,12 +1697,17 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { commitHookEffectListUnmount( HookLayout | HookHasEffect, finishedWork, + finishedWork.return, ); } finally { recordLayoutEffectDuration(finishedWork); } } else { - commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } return; } @@ -1701,12 +1761,20 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { ) { try { startLayoutEffectTimer(); - commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } finally { recordLayoutEffectDuration(finishedWork); } } else { - commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } return; } @@ -1958,17 +2026,18 @@ function commitMutationEffects_begin( null, root, childToDelete, + fiber, renderPriorityLevel, ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(childToDelete, error); + captureCommitPhaseError(childToDelete, fiber, error); } } else { try { - commitDeletion(root, childToDelete, renderPriorityLevel); + commitDeletion(root, childToDelete, fiber, renderPriorityLevel); } catch (error) { - captureCommitPhaseError(childToDelete, error); + captureCommitPhaseError(childToDelete, fiber, error); } } } @@ -2002,14 +2071,14 @@ function commitMutationEffects_complete( ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitMutationEffectsOnFiber(fiber, root, renderPriorityLevel); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } @@ -2144,14 +2213,14 @@ function commitLayoutMountEffects_complete( ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitLayoutEffectOnFiber(root, current, fiber, committedLanes); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2211,14 +2280,14 @@ function commitPassiveMountEffects_complete( ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitPassiveMountOnFiber(root, fiber); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2282,7 +2351,10 @@ function commitPassiveUnmountEffects_begin() { for (let i = 0; i < deletions.length; i++) { const fiberToDelete = deletions[i]; nextEffect = fiberToDelete; - commitPassiveUnmountEffectsInsideOfDeletedTree_begin(fiberToDelete); + commitPassiveUnmountEffectsInsideOfDeletedTree_begin( + fiberToDelete, + fiber, + ); // Now that passive effects have been processed, it's safe to detach lingering pointers. const alternate = fiberToDelete.alternate; @@ -2343,10 +2415,18 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { finishedWork.mode & ProfileMode ) { startPassiveEffectTimer(); - commitHookEffectListUnmount(HookPassive | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + finishedWork, + finishedWork.return, + ); recordPassiveEffectDuration(finishedWork); } else { - commitHookEffectListUnmount(HookPassive | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + finishedWork, + finishedWork.return, + ); } break; } @@ -2355,6 +2435,7 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( deletedSubtreeRoot: Fiber, + nearestMountedAncestor: Fiber | null, ) { while (nextEffect !== null) { const fiber = nextEffect; @@ -2362,7 +2443,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( // Deletion effects fire in parent -> child order // TODO: Check if fiber has a PassiveStatic flag setCurrentDebugFiberInDEV(fiber); - commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber, nearestMountedAncestor); resetCurrentDebugFiberInDEV(); const child = fiber.child; @@ -2399,7 +2480,10 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( } } -function commitPassiveUnmountInsideDeletedTreeOnFiber(current: Fiber): void { +function commitPassiveUnmountInsideDeletedTreeOnFiber( + current: Fiber, + nearestMountedAncestor: Fiber | null, +): void { switch (current.tag) { case FunctionComponent: case ForwardRef: @@ -2410,10 +2494,18 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(current: Fiber): void { current.mode & ProfileMode ) { startPassiveEffectTimer(); - commitHookEffectListUnmount(HookPassive, current); + commitHookEffectListUnmount( + HookPassive, + current, + nearestMountedAncestor, + ); recordPassiveEffectDuration(current); } else { - commitHookEffectListUnmount(HookPassive, current); + commitHookEffectListUnmount( + HookPassive, + current, + nearestMountedAncestor, + ); } break; } @@ -2454,7 +2546,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, mountError); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2463,7 +2555,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { invokeGuardedCallback(null, instance.componentDidMount, instance); if (hasCaughtError()) { const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, mountError); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2488,7 +2580,7 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, mountError); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2514,7 +2606,7 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, unmountError); + captureCommitPhaseError(fiber, fiber.return, unmountError); } break; } @@ -2526,12 +2618,12 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { safelyCallComponentWillUnmount, null, fiber, - instance, fiber.return, + instance, ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, unmountError); + captureCommitPhaseError(fiber, fiber.return, unmountError); } } break; @@ -2558,7 +2650,7 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, unmountError); + captureCommitPhaseError(fiber, fiber.return, unmountError); } break; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 642f6dce02aa8..d5ce4dd688d63 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -177,7 +177,11 @@ const callComponentWillUnmountWithTimer = function(current, instance) { }; // Capture errors so they don't interrupt unmounting. -function safelyCallComponentWillUnmount(current: Fiber, instance: any) { +function safelyCallComponentWillUnmount( + current: Fiber, + nearestMountedAncestor: Fiber | null, + instance: any, +) { if (__DEV__) { invokeGuardedCallback( null, @@ -188,18 +192,18 @@ function safelyCallComponentWillUnmount(current: Fiber, instance: any) { ); 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') { @@ -218,7 +222,7 @@ function safelyDetachRef(current: Fiber) { if (hasCaughtError()) { const refError = clearCaughtError(); - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } else { try { @@ -237,7 +241,7 @@ function safelyDetachRef(current: Fiber) { ref(null); } } catch (refError) { - captureCommitPhaseError(current, refError); + captureCommitPhaseError(current, nearestMountedAncestor, refError); } } } else { @@ -246,18 +250,22 @@ function safelyDetachRef(current: Fiber) { } } -function safelyCallDestroy(current: Fiber, destroy: () => void) { +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); } } } @@ -321,14 +329,14 @@ function commitBeforeMutationEffects_complete() { ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitBeforeMutationEffectsOnFiber(fiber); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } @@ -462,7 +470,11 @@ function commitBeforeMutationEffectsDeletion(deletion: Fiber) { } } -function commitHookEffectListUnmount(flags: HookFlags, finishedWork: Fiber) { +function commitHookEffectListUnmount( + flags: HookFlags, + finishedWork: Fiber, + nearestMountedAncestor: Fiber | null, +) { const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any); const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; if (lastEffect !== null) { @@ -474,7 +486,7 @@ function commitHookEffectListUnmount(flags: HookFlags, finishedWork: Fiber) { const destroy = effect.destroy; effect.destroy = undefined; if (destroy !== undefined) { - safelyCallDestroy(finishedWork, destroy); + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); } } effect = effect.next; @@ -1049,6 +1061,7 @@ function commitDetachRef(current: Fiber) { function commitUnmount( finishedRoot: FiberRoot, current: Fiber, + nearestMountedAncestor: Fiber, renderPriorityLevel: ReactPriorityLevel, ): void { onCommitUnmount(current); @@ -1075,10 +1088,10 @@ function commitUnmount( current.mode & ProfileMode ) { startLayoutEffectTimer(); - safelyCallDestroy(current, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); recordLayoutEffectDuration(current); } else { - safelyCallDestroy(current, destroy); + safelyCallDestroy(current, nearestMountedAncestor, destroy); } } } @@ -1089,15 +1102,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, + nearestMountedAncestor, + instance, + ); } return; } case HostComponent: { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); return; } case HostPortal: { @@ -1105,7 +1122,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); } @@ -1135,7 +1157,7 @@ function commitUnmount( } case ScopeComponent: { if (enableScopeAPI) { - safelyDetachRef(current); + safelyDetachRef(current, nearestMountedAncestor); } return; } @@ -1145,6 +1167,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 @@ -1154,7 +1177,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 ( @@ -1455,6 +1483,7 @@ function insertOrAppendPlacementNode( function unmountHostComponents( 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 @@ -1504,7 +1533,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) { @@ -1521,7 +1555,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) { @@ -1573,7 +1612,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; @@ -1603,15 +1647,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); @@ -1642,12 +1697,17 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { commitHookEffectListUnmount( HookLayout | HookHasEffect, finishedWork, + finishedWork.return, ); } finally { recordLayoutEffectDuration(finishedWork); } } else { - commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } return; } @@ -1701,12 +1761,20 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { ) { try { startLayoutEffectTimer(); - commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } finally { recordLayoutEffectDuration(finishedWork); } } else { - commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } return; } @@ -1958,17 +2026,18 @@ function commitMutationEffects_begin( null, root, childToDelete, + fiber, renderPriorityLevel, ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(childToDelete, error); + captureCommitPhaseError(childToDelete, fiber, error); } } else { try { - commitDeletion(root, childToDelete, renderPriorityLevel); + commitDeletion(root, childToDelete, fiber, renderPriorityLevel); } catch (error) { - captureCommitPhaseError(childToDelete, error); + captureCommitPhaseError(childToDelete, fiber, error); } } } @@ -2002,14 +2071,14 @@ function commitMutationEffects_complete( ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitMutationEffectsOnFiber(fiber, root, renderPriorityLevel); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } @@ -2144,14 +2213,14 @@ function commitLayoutMountEffects_complete( ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitLayoutEffectOnFiber(root, current, fiber, committedLanes); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2211,14 +2280,14 @@ function commitPassiveMountEffects_complete( ); if (hasCaughtError()) { const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } resetCurrentDebugFiberInDEV(); } else { try { commitPassiveMountOnFiber(root, fiber); } catch (error) { - captureCommitPhaseError(fiber, error); + captureCommitPhaseError(fiber, fiber.return, error); } } } @@ -2282,7 +2351,10 @@ function commitPassiveUnmountEffects_begin() { for (let i = 0; i < deletions.length; i++) { const fiberToDelete = deletions[i]; nextEffect = fiberToDelete; - commitPassiveUnmountEffectsInsideOfDeletedTree_begin(fiberToDelete); + commitPassiveUnmountEffectsInsideOfDeletedTree_begin( + fiberToDelete, + fiber, + ); // Now that passive effects have been processed, it's safe to detach lingering pointers. const alternate = fiberToDelete.alternate; @@ -2343,10 +2415,18 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { finishedWork.mode & ProfileMode ) { startPassiveEffectTimer(); - commitHookEffectListUnmount(HookPassive | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + finishedWork, + finishedWork.return, + ); recordPassiveEffectDuration(finishedWork); } else { - commitHookEffectListUnmount(HookPassive | HookHasEffect, finishedWork); + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + finishedWork, + finishedWork.return, + ); } break; } @@ -2355,6 +2435,7 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( deletedSubtreeRoot: Fiber, + nearestMountedAncestor: Fiber | null, ) { while (nextEffect !== null) { const fiber = nextEffect; @@ -2362,7 +2443,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( // Deletion effects fire in parent -> child order // TODO: Check if fiber has a PassiveStatic flag setCurrentDebugFiberInDEV(fiber); - commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber, nearestMountedAncestor); resetCurrentDebugFiberInDEV(); const child = fiber.child; @@ -2399,7 +2480,10 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( } } -function commitPassiveUnmountInsideDeletedTreeOnFiber(current: Fiber): void { +function commitPassiveUnmountInsideDeletedTreeOnFiber( + current: Fiber, + nearestMountedAncestor: Fiber | null, +): void { switch (current.tag) { case FunctionComponent: case ForwardRef: @@ -2410,10 +2494,18 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(current: Fiber): void { current.mode & ProfileMode ) { startPassiveEffectTimer(); - commitHookEffectListUnmount(HookPassive, current); + commitHookEffectListUnmount( + HookPassive, + current, + nearestMountedAncestor, + ); recordPassiveEffectDuration(current); } else { - commitHookEffectListUnmount(HookPassive, current); + commitHookEffectListUnmount( + HookPassive, + current, + nearestMountedAncestor, + ); } break; } @@ -2454,7 +2546,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, mountError); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2463,7 +2555,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { invokeGuardedCallback(null, instance.componentDidMount, instance); if (hasCaughtError()) { const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, mountError); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2488,7 +2580,7 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, mountError); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2514,7 +2606,7 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, unmountError); + captureCommitPhaseError(fiber, fiber.return, unmountError); } break; } @@ -2526,12 +2618,12 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { safelyCallComponentWillUnmount, null, fiber, - instance, fiber.return, + instance, ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, unmountError); + captureCommitPhaseError(fiber, fiber.return, unmountError); } } break; @@ -2558,7 +2650,7 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { ); if (hasCaughtError()) { const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, unmountError); + captureCommitPhaseError(fiber, fiber.return, unmountError); } break; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index d83cf9db9389d..59fce3d254f0b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -33,6 +33,7 @@ import { enableSchedulingProfiler, disableSchedulerTimeoutInWorkLoop, enableDoubleInvokingEffects, + skipUnmountedBoundaries, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -2312,7 +2313,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. @@ -2320,7 +2325,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/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 699b47596d0c8..833fa0e98b4fc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -33,6 +33,7 @@ import { enableSchedulingProfiler, disableSchedulerTimeoutInWorkLoop, enableDoubleInvokingEffects, + skipUnmountedBoundaries, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -2292,7 +2293,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. @@ -2300,7 +2305,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 489928f27d208..a792f8609be44 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2358,6 +2358,210 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([]); }); + describe('errors thrown in passive destroy function within unmounted trees', () => { + let BrokenUseEffectCleanup; + let ErrorBoundary; + let LogOnlyErrorBoundary; + + beforeEach(() => { + BrokenUseEffectCleanup = function() { + useEffect(() => { + Scheduler.unstable_yieldValue('BrokenUseEffectCleanup useEffect'); + return () => { + Scheduler.unstable_yieldValue( + 'BrokenUseEffectCleanup useEffect destroy', + ); + throw new Error('Expected error'); + }; + }, []); + + return 'inner child'; + }; + + ErrorBoundary = class extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + Scheduler.unstable_yieldValue( + `ErrorBoundary static getDerivedStateFromError`, + ); + return {error}; + } + componentDidCatch(error, info) { + Scheduler.unstable_yieldValue(`ErrorBoundary componentDidCatch`); + } + render() { + if (this.state.error) { + Scheduler.unstable_yieldValue('ErrorBoundary render error'); + return ; + } + Scheduler.unstable_yieldValue('ErrorBoundary render success'); + return this.props.children || null; + } + }; + + LogOnlyErrorBoundary = class extends React.Component { + componentDidCatch(error, info) { + Scheduler.unstable_yieldValue( + `LogOnlyErrorBoundary componentDidCatch`, + ); + } + render() { + Scheduler.unstable_yieldValue(`LogOnlyErrorBoundary render`); + return this.props.children || null; + } + }; + }); + + // @gate skipUnmountedBoundaries + it('should use the nearest still-mounted boundary if there are no unmounted boundaries', () => { + act(() => { + ReactNoop.render( + + + , + ); + }); + + expect(Scheduler).toHaveYielded([ + 'LogOnlyErrorBoundary render', + 'BrokenUseEffectCleanup useEffect', + ]); + + act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'LogOnlyErrorBoundary render', + 'BrokenUseEffectCleanup useEffect destroy', + 'LogOnlyErrorBoundary componentDidCatch', + ]); + }); + + // @gate skipUnmountedBoundaries + it('should skip unmounted boundaries and use the nearest still-mounted boundary', () => { + function Conditional({showChildren}) { + if (showChildren) { + return ( + + + + ); + } else { + return null; + } + } + + act(() => { + ReactNoop.render( + + + , + ); + }); + + expect(Scheduler).toHaveYielded([ + 'LogOnlyErrorBoundary render', + 'ErrorBoundary render success', + 'BrokenUseEffectCleanup useEffect', + ]); + + act(() => { + ReactNoop.render( + + + , + ); + }); + + expect(Scheduler).toHaveYielded([ + 'LogOnlyErrorBoundary render', + 'BrokenUseEffectCleanup useEffect destroy', + 'LogOnlyErrorBoundary componentDidCatch', + ]); + }); + + // @gate skipUnmountedBoundaries + it('should call getDerivedStateFromError in the nearest still-mounted boundary', () => { + function Conditional({showChildren}) { + if (showChildren) { + return ; + } else { + return null; + } + } + + act(() => { + ReactNoop.render( + + + , + ); + }); + + expect(Scheduler).toHaveYielded([ + 'ErrorBoundary render success', + 'BrokenUseEffectCleanup useEffect', + ]); + + act(() => { + ReactNoop.render( + + + , + ); + }); + + expect(Scheduler).toHaveYielded([ + 'ErrorBoundary render success', + 'BrokenUseEffectCleanup useEffect destroy', + 'ErrorBoundary static getDerivedStateFromError', + 'ErrorBoundary render error', + 'ErrorBoundary componentDidCatch', + ]); + + expect(ReactNoop.getChildren()).toEqual([ + span('ErrorBoundary fallback'), + ]); + }); + + // @gate skipUnmountedBoundaries + it('should rethrow error if there are no still-mounted boundaries', () => { + function Conditional({showChildren}) { + if (showChildren) { + return ( + + + + ); + } else { + return null; + } + } + + act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'ErrorBoundary render success', + 'BrokenUseEffectCleanup useEffect', + ]); + + expect(() => { + act(() => { + ReactNoop.render(); + }); + }).toThrow('Expected error'); + + expect(Scheduler).toHaveYielded([ + 'BrokenUseEffectCleanup useEffect destroy', + ]); + + expect(ReactNoop.getChildren()).toEqual([]); + }); + }); + it('calls passive effect destroy functions for memoized components', () => { const Wrapper = ({children}) => children; function Child() { @@ -2594,6 +2798,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 c3ffb65e96ff2..8611a9da06bc6 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -101,6 +101,12 @@ export const enableNewReconciler = false; export const disableNativeComponentFrames = 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 23c926c65a3fb..0fd02c2fd3f18 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -46,6 +46,7 @@ export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = 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 3ed2d7701c93f..c76048a8b2b44 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -45,6 +45,7 @@ export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = 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 c6ffacb27bab2..447d77effbc4e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -45,6 +45,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = 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 84ea8902f60c2..6a6d8f39c59b4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -45,6 +45,7 @@ export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = 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 eb1630fc49bd7..56e7dde89dfbd 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -45,6 +45,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = 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 87133385439f3..537c3fbf1c090 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -45,6 +45,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = 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 671f44be02952..24dd557047005 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -45,6 +45,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = true; 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 cf0ca5ae54393..cf858b767cc3b 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 8b2bda551339e..85ce6eca57284 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, enableDoubleInvokingEffects, enableUseRefAccessWarning, disableNativeComponentFrames,