diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 4c46eb7d0da92..6c3de69780467 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -408,6 +408,49 @@ describe('ReactDOM', () => { } }); + it('throws in DEV if jsdom is destroyed by the time setState() is called', () => { + class App extends React.Component { + state = {x: 1}; + componentDidUpdate() {} + render() { + return
; + } + } + const container = document.createElement('div'); + const instance = ReactDOM.render(, container); + const documentDescriptor = Object.getOwnPropertyDescriptor( + global, + 'document', + ); + try { + // Emulate jsdom environment cleanup. + // This is roughly what happens if the test finished and then + // an asynchronous callback tried to setState() after this. + delete global.document; + + // The error we're interested in is thrown by invokeGuardedCallback, which + // in DEV is used 1) to replay a failed begin phase, or 2) when calling + // lifecycle methods. We're triggering the second case here. + const fn = () => instance.setState({x: 2}); + if (__DEV__) { + expect(fn).toThrow( + 'The `document` global was defined when React was initialized, but is not ' + + 'defined anymore. This can happen in a test environment if a component ' + + 'schedules an update from an asynchronous callback, but the test has already ' + + 'finished running. To solve this, you can either unmount the component at ' + + 'the end of your test (and ensure that any asynchronous operations get ' + + 'canceled in `componentWillUnmount`), or you can change the test itself ' + + 'to be asynchronous.', + ); + } else { + expect(fn).not.toThrow(); + } + } finally { + // Don't break other tests. + Object.defineProperty(global, 'document', documentDescriptor); + } + }); + it('reports stacks with re-entrant renderToString() calls on the client', () => { function Child2(props) { return {props.children}; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 60158f1f4aaa9..fe1bcfa640b18 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -57,6 +57,11 @@ import { OffscreenComponent, LegacyHiddenComponent, } from './ReactWorkTags'; +import { + invokeGuardedCallback, + hasCaughtError, + clearCaughtError, +} from 'shared/ReactErrorUtils'; import {detachDeletedInstance} from './ReactFiberHostConfig'; import { NoFlags, @@ -184,10 +189,24 @@ function safelyCallCommitHookLayoutEffectListMount( current: Fiber, nearestMountedAncestor: Fiber | null, ) { - try { - commitHookEffectListMount(HookLayout, current); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookLayout, + current, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + commitHookEffectListMount(HookLayout, current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } @@ -197,10 +216,24 @@ function safelyCallComponentWillUnmount( nearestMountedAncestor: Fiber | null, instance: any, ) { - try { - callComponentWillUnmountWithTimer(current, instance); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback( + null, + callComponentWillUnmountWithTimer, + null, + current, + instance, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + callComponentWillUnmountWithTimer(current, instance); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } @@ -210,19 +243,35 @@ function safelyCallComponentDidMount( nearestMountedAncestor: Fiber | null, instance: any, ) { - try { - instance.componentDidMount(); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback(null, instance.componentDidMount, instance); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + instance.componentDidMount(); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } // Capture errors so they don't interrupt mounting. function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { - try { - commitAttachRef(current); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback(null, commitAttachRef, null, current); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + commitAttachRef(current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } @@ -230,23 +279,42 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; if (ref !== null) { if (typeof ref === 'function') { - try { + if (__DEV__) { if ( enableProfilerTimer && enableProfilerCommitHooks && current.mode & ProfileMode ) { - try { - startLayoutEffectTimer(); + startLayoutEffectTimer(); + invokeGuardedCallback(null, ref, null, null); + recordLayoutEffectDuration(current); + } else { + invokeGuardedCallback(null, ref, null, null); + } + + if (hasCaughtError()) { + const refError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, refError); + } + } else { + try { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + current.mode & ProfileMode + ) { + try { + startLayoutEffectTimer(); + ref(null); + } finally { + recordLayoutEffectDuration(current); + } + } else { ref(null); - } finally { - recordLayoutEffectDuration(current); } - } else { - ref(null); + } catch (refError) { + captureCommitPhaseError(current, nearestMountedAncestor, refError); } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); } } else { ref.current = null; @@ -259,10 +327,18 @@ function safelyCallDestroy( nearestMountedAncestor: Fiber | null, destroy: () => void, ) { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); + if (__DEV__) { + invokeGuardedCallback(null, destroy, null); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } else { + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } } } @@ -319,13 +395,26 @@ function commitBeforeMutationEffects_begin() { function commitBeforeMutationEffects_complete() { while (nextEffect !== null) { const fiber = nextEffect; - setCurrentDebugFiberInDEV(fiber); - try { - commitBeforeMutationEffectsOnFiber(fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitBeforeMutationEffectsOnFiber, + null, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitBeforeMutationEffectsOnFiber(fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2062,10 +2151,25 @@ function commitMutationEffects_begin(root: FiberRoot) { if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const childToDelete = deletions[i]; - try { - commitDeletion(root, childToDelete, fiber); - } catch (error) { - captureCommitPhaseError(childToDelete, fiber, error); + if (__DEV__) { + invokeGuardedCallback( + null, + commitDeletion, + null, + root, + childToDelete, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(childToDelete, fiber, error); + } + } else { + try { + commitDeletion(root, childToDelete, fiber); + } catch (error) { + captureCommitPhaseError(childToDelete, fiber, error); + } } } } @@ -2083,13 +2187,27 @@ function commitMutationEffects_begin(root: FiberRoot) { function commitMutationEffects_complete(root: FiberRoot) { while (nextEffect !== null) { const fiber = nextEffect; - setCurrentDebugFiberInDEV(fiber); - try { - commitMutationEffectsOnFiber(fiber, root); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitMutationEffectsOnFiber, + null, + fiber, + root, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitMutationEffectsOnFiber(fiber, root); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2325,13 +2443,29 @@ function commitLayoutMountEffects_complete( } } else if ((fiber.flags & LayoutMask) !== NoFlags) { const current = fiber.alternate; - setCurrentDebugFiberInDEV(fiber); - try { - commitLayoutEffectOnFiber(root, current, fiber, committedLanes); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitLayoutEffectOnFiber, + null, + root, + current, + fiber, + committedLanes, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitLayoutEffectOnFiber(root, current, fiber, committedLanes); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2378,13 +2512,27 @@ function commitPassiveMountEffects_complete( while (nextEffect !== null) { const fiber = nextEffect; if ((fiber.flags & Passive) !== NoFlags) { - setCurrentDebugFiberInDEV(fiber); - try { - commitPassiveMountOnFiber(root, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitPassiveMountOnFiber, + null, + root, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitPassiveMountOnFiber(root, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2661,19 +2809,25 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListMount(HookLayout | HookHasEffect, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookLayout | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } case ClassComponent: { const instance = fiber.stateNode; - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback(null, instance.componentDidMount, instance); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2689,10 +2843,16 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListMount(HookPassive | HookHasEffect, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookPassive | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2708,21 +2868,35 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListUnmount( - HookLayout | HookHasEffect, - fiber, - fiber.return, - ); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); } break; } case ClassComponent: { const instance = fiber.stateNode; if (typeof instance.componentWillUnmount === 'function') { - safelyCallComponentWillUnmount(fiber, fiber.return, instance); + invokeGuardedCallback( + null, + safelyCallComponentWillUnmount, + null, + fiber, + fiber.return, + instance, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); + } } break; } @@ -2738,15 +2912,19 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListUnmount( - HookPassive | HookHasEffect, - fiber, - fiber.return, - ); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + 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 c741023f22bf5..773d85445cd7c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -57,6 +57,11 @@ import { OffscreenComponent, LegacyHiddenComponent, } from './ReactWorkTags'; +import { + invokeGuardedCallback, + hasCaughtError, + clearCaughtError, +} from 'shared/ReactErrorUtils'; import {detachDeletedInstance} from './ReactFiberHostConfig'; import { NoFlags, @@ -184,10 +189,24 @@ function safelyCallCommitHookLayoutEffectListMount( current: Fiber, nearestMountedAncestor: Fiber | null, ) { - try { - commitHookEffectListMount(HookLayout, current); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookLayout, + current, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + commitHookEffectListMount(HookLayout, current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } @@ -197,10 +216,24 @@ function safelyCallComponentWillUnmount( nearestMountedAncestor: Fiber | null, instance: any, ) { - try { - callComponentWillUnmountWithTimer(current, instance); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback( + null, + callComponentWillUnmountWithTimer, + null, + current, + instance, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + callComponentWillUnmountWithTimer(current, instance); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } @@ -210,19 +243,35 @@ function safelyCallComponentDidMount( nearestMountedAncestor: Fiber | null, instance: any, ) { - try { - instance.componentDidMount(); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback(null, instance.componentDidMount, instance); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + instance.componentDidMount(); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } // Capture errors so they don't interrupt mounting. function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { - try { - commitAttachRef(current); - } catch (unmountError) { - captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + if (__DEV__) { + invokeGuardedCallback(null, commitAttachRef, null, current); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } + } else { + try { + commitAttachRef(current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); + } } } @@ -230,23 +279,42 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; if (ref !== null) { if (typeof ref === 'function') { - try { + if (__DEV__) { if ( enableProfilerTimer && enableProfilerCommitHooks && current.mode & ProfileMode ) { - try { - startLayoutEffectTimer(); + startLayoutEffectTimer(); + invokeGuardedCallback(null, ref, null, null); + recordLayoutEffectDuration(current); + } else { + invokeGuardedCallback(null, ref, null, null); + } + + if (hasCaughtError()) { + const refError = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, refError); + } + } else { + try { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + current.mode & ProfileMode + ) { + try { + startLayoutEffectTimer(); + ref(null); + } finally { + recordLayoutEffectDuration(current); + } + } else { ref(null); - } finally { - recordLayoutEffectDuration(current); } - } else { - ref(null); + } catch (refError) { + captureCommitPhaseError(current, nearestMountedAncestor, refError); } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); } } else { ref.current = null; @@ -259,10 +327,18 @@ function safelyCallDestroy( nearestMountedAncestor: Fiber | null, destroy: () => void, ) { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); + if (__DEV__) { + invokeGuardedCallback(null, destroy, null); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } else { + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } } } @@ -319,13 +395,26 @@ function commitBeforeMutationEffects_begin() { function commitBeforeMutationEffects_complete() { while (nextEffect !== null) { const fiber = nextEffect; - setCurrentDebugFiberInDEV(fiber); - try { - commitBeforeMutationEffectsOnFiber(fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitBeforeMutationEffectsOnFiber, + null, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitBeforeMutationEffectsOnFiber(fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2062,10 +2151,25 @@ function commitMutationEffects_begin(root: FiberRoot) { if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const childToDelete = deletions[i]; - try { - commitDeletion(root, childToDelete, fiber); - } catch (error) { - captureCommitPhaseError(childToDelete, fiber, error); + if (__DEV__) { + invokeGuardedCallback( + null, + commitDeletion, + null, + root, + childToDelete, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(childToDelete, fiber, error); + } + } else { + try { + commitDeletion(root, childToDelete, fiber); + } catch (error) { + captureCommitPhaseError(childToDelete, fiber, error); + } } } } @@ -2083,13 +2187,27 @@ function commitMutationEffects_begin(root: FiberRoot) { function commitMutationEffects_complete(root: FiberRoot) { while (nextEffect !== null) { const fiber = nextEffect; - setCurrentDebugFiberInDEV(fiber); - try { - commitMutationEffectsOnFiber(fiber, root); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitMutationEffectsOnFiber, + null, + fiber, + root, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitMutationEffectsOnFiber(fiber, root); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2325,13 +2443,29 @@ function commitLayoutMountEffects_complete( } } else if ((fiber.flags & LayoutMask) !== NoFlags) { const current = fiber.alternate; - setCurrentDebugFiberInDEV(fiber); - try { - commitLayoutEffectOnFiber(root, current, fiber, committedLanes); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitLayoutEffectOnFiber, + null, + root, + current, + fiber, + committedLanes, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitLayoutEffectOnFiber(root, current, fiber, committedLanes); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2378,13 +2512,27 @@ function commitPassiveMountEffects_complete( while (nextEffect !== null) { const fiber = nextEffect; if ((fiber.flags & Passive) !== NoFlags) { - setCurrentDebugFiberInDEV(fiber); - try { - commitPassiveMountOnFiber(root, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitPassiveMountOnFiber, + null, + root, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitPassiveMountOnFiber(root, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } } - resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2661,19 +2809,25 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListMount(HookLayout | HookHasEffect, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookLayout | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } case ClassComponent: { const instance = fiber.stateNode; - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback(null, instance.componentDidMount, instance); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2689,10 +2843,16 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListMount(HookPassive | HookHasEffect, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookPassive | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); } break; } @@ -2708,21 +2868,35 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListUnmount( - HookLayout | HookHasEffect, - fiber, - fiber.return, - ); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); } break; } case ClassComponent: { const instance = fiber.stateNode; if (typeof instance.componentWillUnmount === 'function') { - safelyCallComponentWillUnmount(fiber, fiber.return, instance); + invokeGuardedCallback( + null, + safelyCallComponentWillUnmount, + null, + fiber, + fiber.return, + instance, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); + } } break; } @@ -2738,15 +2912,19 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListUnmount( - HookPassive | HookHasEffect, - fiber, - fiber.return, - ); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); } + break; } } } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js index 1157bb27ebd79..38f32c9f494de 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js @@ -244,4 +244,69 @@ describe('ReactSuspense', () => { expect(ops1).toEqual([]); expect(ops2).toEqual([]); }); + + if (__DEV__) { + // @gate www + it('regression test for #16215 that relies on implementation details', async () => { + // Regression test for https://github.com/facebook/react/pull/16215. + // The bug only happens if there's an error earlier in the commit phase. + // The first error is the one that gets thrown, so to observe the later + // error, I've mocked the ReactErrorUtils module. + // + // If this test starts failing because the implementation details change, + // you can probably just delete it. It's not worth the hassle. + jest.resetModules(); + + const errors = []; + let hasCaughtError = false; + jest.mock('shared/ReactErrorUtils', () => ({ + invokeGuardedCallback(name, fn, context, ...args) { + try { + return fn.call(context, ...args); + } catch (error) { + hasCaughtError = true; + errors.push(error); + } + }, + hasCaughtError() { + return hasCaughtError; + }, + clearCaughtError() { + hasCaughtError = false; + return errors[errors.length - 1]; + }, + })); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + + const {useEffect} = React; + const {PromiseComp} = createThenable(); + function App() { + useEffect(() => { + Scheduler.unstable_yieldValue('Passive Effect'); + }); + return ( + { + throw Error('Oops!'); + }} + fallback="Loading..."> + + + ); + } + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + expect(Scheduler).toFlushAndThrow('Oops!'); + }); + + // Should have only received a single error. Before the bug fix, there was + // also a second error related to the Suspense update queue. + expect(errors.length).toBe(1); + expect(errors[0].message).toEqual('Oops!'); + }); + } });