Move error logging to update callback

This prevents double logging for gDSFE boundaries with createRoot.
facebook · Jun 23, 2021 · d1a3291 · gaearon · Jun 23, 2021 · gaearon
1 parent 8dc9c27
commit d1a3291
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 29 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js
@@ -260,12 +260,6 @@ describe('ReactDOMConsoleErrorReporting', () => {
               message: 'Boom',
             }),
           ],
-          [
-            // Addendum by React:
-            expect.stringContaining(
-              'The above error occurred in the <Foo> component',
-            ),
-          ],
           [
             // TODO: This is duplicated only with createRoot. Why?
             expect.stringContaining('Error: Uncaught [Error: Boom]'),
@@ -274,7 +268,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
             }),
           ],
           [
-            // TODO: This is duplicated only with createRoot. Why?
+            // Addendum by React:
             expect.stringContaining(
               'The above error occurred in the <Foo> component',
             ),
@@ -291,12 +285,6 @@ describe('ReactDOMConsoleErrorReporting', () => {
               message: 'Boom',
             }),
           ],
-          [
-            // TODO: This is duplicated only with createRoot. Why?
-            expect.objectContaining({
-              message: 'Boom',
-            }),
-          ],
         ]);
       }
 

diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js
@@ -108,9 +108,14 @@ function createClassErrorUpdate(
   if (typeof getDerivedStateFromError === 'function') {
     const error = errorInfo.value;
     update.payload = () => {
-      logCapturedError(fiber, errorInfo);
       return getDerivedStateFromError(error);
     };
+    update.callback = () => {
+      if (__DEV__) {
+        markFailedErrorBoundaryForHotReloading(fiber);
+      }
+      logCapturedError(fiber, errorInfo);
+    };
   }
 
   const inst = fiber.stateNode;
@@ -119,16 +124,14 @@ function createClassErrorUpdate(
       if (__DEV__) {
         markFailedErrorBoundaryForHotReloading(fiber);
       }
+      logCapturedError(fiber, errorInfo);
       if (typeof getDerivedStateFromError !== 'function') {
         // To preserve the preexisting retry behavior of error boundaries,
         // we keep track of which ones already failed during this batch.
         // This gets reset before we yield back to the browser.
         // TODO: Warn in strict mode if getDerivedStateFromError is
         // not defined.
         markLegacyErrorBoundaryAsFailed(this);
-
-        // Only log here if componentDidCatch is the only error boundary method defined
-        logCapturedError(fiber, errorInfo);
       }
       const error = errorInfo.value;
       const stack = errorInfo.stack;
@@ -150,10 +153,6 @@ function createClassErrorUpdate(
         }
       }
     };
-  } else if (__DEV__) {
-    update.callback = () => {
-      markFailedErrorBoundaryForHotReloading(fiber);
-    };
   }
   return update;
 }

diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js
@@ -108,9 +108,14 @@ function createClassErrorUpdate(
   if (typeof getDerivedStateFromError === 'function') {
     const error = errorInfo.value;
     update.payload = () => {
-      logCapturedError(fiber, errorInfo);
       return getDerivedStateFromError(error);
     };
+    update.callback = () => {
+      if (__DEV__) {
+        markFailedErrorBoundaryForHotReloading(fiber);
+      }
+      logCapturedError(fiber, errorInfo);
+    };
   }
 
   const inst = fiber.stateNode;
@@ -119,16 +124,14 @@ function createClassErrorUpdate(
       if (__DEV__) {
         markFailedErrorBoundaryForHotReloading(fiber);
       }
+      logCapturedError(fiber, errorInfo);
       if (typeof getDerivedStateFromError !== 'function') {
         // To preserve the preexisting retry behavior of error boundaries,
         // we keep track of which ones already failed during this batch.
         // This gets reset before we yield back to the browser.
         // TODO: Warn in strict mode if getDerivedStateFromError is
         // not defined.
         markLegacyErrorBoundaryAsFailed(this);
-
-        // Only log here if componentDidCatch is the only error boundary method defined
-        logCapturedError(fiber, errorInfo);
       }
       const error = errorInfo.value;
       const stack = errorInfo.stack;
@@ -150,10 +153,6 @@ function createClassErrorUpdate(
         }
       }
     };
-  } else if (__DEV__) {
-    update.callback = () => {
-      markFailedErrorBoundaryForHotReloading(fiber);
-    };
   }
   return update;
 }