Repository: facebook/react
Pull request: Remove invokeGuardedCallback from commit phase

Alternative to https://github.com/facebook/react/pull/21658.

The reason we have this in the first place is to make "pause on uncaught exceptions" work. That's a development feature. However, the way we do this now, especially after the effect traversal refactor, causes quite a bit of a DEV slowdown in commit phase. This makes it harder to have an accurate sense of how slow a render/commit is. While DEV isn't meant to be representative of real perf, it's bad that commits seem disproportionally expensive because of this. It also legitimately slows down the app in development on its own. So we have a devx fix making devx worse. Which is important? I'd say it's not worth it.

Seb proposed that we just remove this and use the prod path in the commit phase. This PR does that.

## Example DEV perf difference

This is on a very shallow tree (1 level deep) but with many host children.

Commit before: 10ms

<img width="304" alt="Screenshot 2021-06-11 at 22 42 24" src="https://user-images.githubusercontent.com/810438/121751813-d0e76500-cb06-11eb-99b9-adc237a53d7e.png">

Commit after: 0.1ms

<img width="410" alt="Screenshot 2021-06-11 at 22 41 49" src="https://user-images.githubusercontent.com/810438/121751869-eb214300-cb06-11eb-81d3-32308a17a805.png">

I expect this to also show up in deeper trees, especially the ones with lots of effects etc.

Changed files:
filename: packages/react-dom/src/__tests__/ReactDOM-test.js
patch: @@ -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 <div />;
-      }
-    }
-    const container = document.createElement('div');
-    const instance = ReactDOM.render(<App />, 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 <span ariaTypo3="no">{props.children}</span>;

filename: packages/react-reconciler/src/ReactFiberCommitWork.new.js
patch: @@ -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,78 +210,43 @@ 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);
   }
 }
 
 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;
       }
     }
   }

filename: packages/react-reconciler/src/ReactFiberCommitWork.old.js
patch: @@ -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,78 +210,43 @@ 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);
   }
 }
 
 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;
       }
     }
   }

filename: packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js
patch: @@ -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 (
-          <React.Suspense
-            suspenseCallback={() => {
-              throw Error('Oops!');
-            }}
-            fallback="Loading...">
-            <PromiseComp />
-          </React.Suspense>
-        );
-      }
-      const root = ReactNoop.createRoot();
-      await ReactNoop.act(async () => {
-        root.render(<App />);
-        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!');
-    });
-  }
 });