From a8f5e77b921c890c215fb1c7e24a06f38598deb1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Jun 2021 21:45:53 +0100 Subject: [PATCH] Remove invokeGuardedCallback from commit phase (#21666) * Remove invokeGuardedCallback from commit phase * Sync fork --- .../react-dom/src/__tests__/ReactDOM-test.js | 43 --- .../src/ReactFiberCommitWork.new.js | 350 +++++------------- .../src/ReactFiberCommitWork.old.js | 350 +++++------------- .../__tests__/ReactSuspenseCallback-test.js | 65 ---- 4 files changed, 172 insertions(+), 636 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 6c3de6978046..4c46eb7d0da9 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -408,49 +408,6 @@ 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 37c9bbcda79d..e31ed8577938 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -57,11 +57,6 @@ import { OffscreenComponent, LegacyHiddenComponent, } from './ReactWorkTags'; -import { - invokeGuardedCallback, - hasCaughtError, - clearCaughtError, -} from 'shared/ReactErrorUtils'; import {detachDeletedInstance} from './ReactFiberHostConfig'; import { NoFlags, @@ -189,24 +184,10 @@ function safelyCallCommitHookLayoutEffectListMount( current: Fiber, nearestMountedAncestor: Fiber | null, ) { - 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); - } + try { + commitHookEffectListMount(HookLayout, current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } @@ -216,24 +197,10 @@ function safelyCallComponentWillUnmount( nearestMountedAncestor: Fiber | null, instance: any, ) { - 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); - } + try { + callComponentWillUnmountWithTimer(current, instance); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } @@ -243,35 +210,19 @@ function safelyCallComponentDidMount( nearestMountedAncestor: Fiber | null, instance: any, ) { - 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); - } + try { + instance.componentDidMount(); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } // Capture errors so they don't interrupt mounting. function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { - 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); - } + try { + commitAttachRef(current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } @@ -279,42 +230,23 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; if (ref !== null) { if (typeof ref === 'function') { - if (__DEV__) { + try { if ( enableProfilerTimer && enableProfilerCommitHooks && current.mode & ProfileMode ) { - 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 { + try { + startLayoutEffectTimer(); ref(null); + } finally { + recordLayoutEffectDuration(current); } - } catch (refError) { - captureCommitPhaseError(current, nearestMountedAncestor, refError); + } else { + ref(null); } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); } } else { ref.current = null; @@ -327,18 +259,10 @@ function safelyCallDestroy( nearestMountedAncestor: Fiber | null, destroy: () => void, ) { - if (__DEV__) { - invokeGuardedCallback(null, destroy, null); - if (hasCaughtError()) { - const error = clearCaughtError(); - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } else { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); } } @@ -395,26 +319,13 @@ function commitBeforeMutationEffects_begin() { function commitBeforeMutationEffects_complete() { while (nextEffect !== null) { const fiber = nextEffect; - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitBeforeMutationEffectsOnFiber(fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2151,25 +2062,10 @@ function commitMutationEffects_begin(root: FiberRoot) { if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const childToDelete = deletions[i]; - 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); - } + try { + commitDeletion(root, childToDelete, fiber); + } catch (error) { + captureCommitPhaseError(childToDelete, fiber, error); } } } @@ -2187,27 +2083,13 @@ function commitMutationEffects_begin(root: FiberRoot) { function commitMutationEffects_complete(root: FiberRoot) { while (nextEffect !== null) { const fiber = nextEffect; - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitMutationEffectsOnFiber(fiber, root); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2444,29 +2326,13 @@ function commitLayoutMountEffects_complete( } } else if ((fiber.flags & LayoutMask) !== NoFlags) { const current = fiber.alternate; - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitLayoutEffectOnFiber(root, current, fiber, committedLanes); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2513,27 +2379,13 @@ function commitPassiveMountEffects_complete( while (nextEffect !== null) { const fiber = nextEffect; if ((fiber.flags & Passive) !== NoFlags) { - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitPassiveMountOnFiber(root, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2810,25 +2662,19 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListMount, - null, - HookLayout | HookHasEffect, - fiber, - ); - if (hasCaughtError()) { - const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, mountError); + try { + commitHookEffectListMount(HookLayout | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } case ClassComponent: { const instance = fiber.stateNode; - invokeGuardedCallback(null, instance.componentDidMount, instance); - if (hasCaughtError()) { - const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, mountError); + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } @@ -2844,16 +2690,10 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListMount, - null, - HookPassive | HookHasEffect, - fiber, - ); - if (hasCaughtError()) { - const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, mountError); + try { + commitHookEffectListMount(HookPassive | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } @@ -2869,35 +2709,21 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListUnmount, - null, - HookLayout | HookHasEffect, - fiber, - fiber.return, - ); - if (hasCaughtError()) { - const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, unmountError); + try { + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } case ClassComponent: { const instance = fiber.stateNode; if (typeof instance.componentWillUnmount === 'function') { - invokeGuardedCallback( - null, - safelyCallComponentWillUnmount, - null, - fiber, - fiber.return, - instance, - ); - if (hasCaughtError()) { - const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, unmountError); - } + safelyCallComponentWillUnmount(fiber, fiber.return, instance); } break; } @@ -2913,19 +2739,15 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListUnmount, - null, - HookPassive | HookHasEffect, - fiber, - fiber.return, - ); - if (hasCaughtError()) { - const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, unmountError); + try { + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } - break; } } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 98e8faebb0b8..4dc06f6f4f7d 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -57,11 +57,6 @@ import { OffscreenComponent, LegacyHiddenComponent, } from './ReactWorkTags'; -import { - invokeGuardedCallback, - hasCaughtError, - clearCaughtError, -} from 'shared/ReactErrorUtils'; import {detachDeletedInstance} from './ReactFiberHostConfig'; import { NoFlags, @@ -189,24 +184,10 @@ function safelyCallCommitHookLayoutEffectListMount( current: Fiber, nearestMountedAncestor: Fiber | null, ) { - 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); - } + try { + commitHookEffectListMount(HookLayout, current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } @@ -216,24 +197,10 @@ function safelyCallComponentWillUnmount( nearestMountedAncestor: Fiber | null, instance: any, ) { - 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); - } + try { + callComponentWillUnmountWithTimer(current, instance); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } @@ -243,35 +210,19 @@ function safelyCallComponentDidMount( nearestMountedAncestor: Fiber | null, instance: any, ) { - 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); - } + try { + instance.componentDidMount(); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } // Capture errors so they don't interrupt mounting. function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { - 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); - } + try { + commitAttachRef(current); + } catch (unmountError) { + captureCommitPhaseError(current, nearestMountedAncestor, unmountError); } } @@ -279,42 +230,23 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; if (ref !== null) { if (typeof ref === 'function') { - if (__DEV__) { + try { if ( enableProfilerTimer && enableProfilerCommitHooks && current.mode & ProfileMode ) { - 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 { + try { + startLayoutEffectTimer(); ref(null); + } finally { + recordLayoutEffectDuration(current); } - } catch (refError) { - captureCommitPhaseError(current, nearestMountedAncestor, refError); + } else { + ref(null); } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); } } else { ref.current = null; @@ -327,18 +259,10 @@ function safelyCallDestroy( nearestMountedAncestor: Fiber | null, destroy: () => void, ) { - if (__DEV__) { - invokeGuardedCallback(null, destroy, null); - if (hasCaughtError()) { - const error = clearCaughtError(); - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } else { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); } } @@ -395,26 +319,13 @@ function commitBeforeMutationEffects_begin() { function commitBeforeMutationEffects_complete() { while (nextEffect !== null) { const fiber = nextEffect; - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitBeforeMutationEffectsOnFiber(fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2151,25 +2062,10 @@ function commitMutationEffects_begin(root: FiberRoot) { if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const childToDelete = deletions[i]; - 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); - } + try { + commitDeletion(root, childToDelete, fiber); + } catch (error) { + captureCommitPhaseError(childToDelete, fiber, error); } } } @@ -2187,27 +2083,13 @@ function commitMutationEffects_begin(root: FiberRoot) { function commitMutationEffects_complete(root: FiberRoot) { while (nextEffect !== null) { const fiber = nextEffect; - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitMutationEffectsOnFiber(fiber, root); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); const sibling = fiber.sibling; if (sibling !== null) { @@ -2444,29 +2326,13 @@ function commitLayoutMountEffects_complete( } } else if ((fiber.flags & LayoutMask) !== NoFlags) { const current = fiber.alternate; - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitLayoutEffectOnFiber(root, current, fiber, committedLanes); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2513,27 +2379,13 @@ function commitPassiveMountEffects_complete( while (nextEffect !== null) { const fiber = nextEffect; if ((fiber.flags & Passive) !== NoFlags) { - 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); - } + setCurrentDebugFiberInDEV(fiber); + try { + commitPassiveMountOnFiber(root, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } + resetCurrentDebugFiberInDEV(); } if (fiber === subtreeRoot) { @@ -2810,25 +2662,19 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListMount, - null, - HookLayout | HookHasEffect, - fiber, - ); - if (hasCaughtError()) { - const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, mountError); + try { + commitHookEffectListMount(HookLayout | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } case ClassComponent: { const instance = fiber.stateNode; - invokeGuardedCallback(null, instance.componentDidMount, instance); - if (hasCaughtError()) { - const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, mountError); + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } @@ -2844,16 +2690,10 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListMount, - null, - HookPassive | HookHasEffect, - fiber, - ); - if (hasCaughtError()) { - const mountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, mountError); + try { + commitHookEffectListMount(HookPassive | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } @@ -2869,35 +2709,21 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListUnmount, - null, - HookLayout | HookHasEffect, - fiber, - fiber.return, - ); - if (hasCaughtError()) { - const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, unmountError); + try { + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } break; } case ClassComponent: { const instance = fiber.stateNode; if (typeof instance.componentWillUnmount === 'function') { - invokeGuardedCallback( - null, - safelyCallComponentWillUnmount, - null, - fiber, - fiber.return, - instance, - ); - if (hasCaughtError()) { - const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, unmountError); - } + safelyCallComponentWillUnmount(fiber, fiber.return, instance); } break; } @@ -2913,19 +2739,15 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - invokeGuardedCallback( - null, - commitHookEffectListUnmount, - null, - HookPassive | HookHasEffect, - fiber, - fiber.return, - ); - if (hasCaughtError()) { - const unmountError = clearCaughtError(); - captureCommitPhaseError(fiber, fiber.return, unmountError); + try { + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); } - break; } } } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js index 38f32c9f494d..1157bb27ebd7 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js @@ -244,69 +244,4 @@ 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!'); - }); - } });