Repository: facebook/react
Pull request: Move error logging to update callback

Fixes some of the logging duplication when `createRoot` is used.
The commit is https://github.com/facebook/react/commit/d1a3291df20e42cf61550bd00a86ad1e526dd0cc.

Basically, wait for the callback instead of logging during the queue processing, same as with cDC boundaries.

Changed files:
filename: packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js
patch: @@ -166,7 +166,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             }),
           ],
           [
-            // TODO: This is duplicated only with createRoot. Why?
+            // This is only duplicated with createRoot
+            // because it retries once with a sync render.
             expect.objectContaining({
               message: 'Boom',
             }),
@@ -181,7 +182,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             }),
           ],
           [
-            // TODO: This is duplicated only with createRoot. Why?
+            // This is only duplicated with createRoot
+            // because it retries once with a sync render.
             expect.stringContaining('Error: Uncaught [Error: Boom]'),
             expect.objectContaining({
               message: 'Boom',
@@ -246,7 +248,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             }),
           ],
           [
-            // TODO: This is duplicated only with createRoot. Why?
+            // This is only duplicated with createRoot
+            // because it retries once with a sync render.
             expect.objectContaining({
               message: 'Boom',
             }),
@@ -261,20 +264,15 @@ describe('ReactDOMConsoleErrorReporting', () => {
             }),
           ],
           [
-            // Addendum by React:
-            expect.stringContaining(
-              'The above error occurred in the <Foo> component',
-            ),
-          ],
-          [
-            // TODO: This is duplicated only with createRoot. Why?
+            // This is only duplicated with createRoot
+            // because it retries once with a sync render.
             expect.stringContaining('Error: Uncaught [Error: Boom]'),
             expect.objectContaining({
               message: 'Boom',
             }),
           ],
           [
-            // TODO: This is duplicated only with createRoot. Why?
+            // Addendum by React:
             expect.stringContaining(
               'The above error occurred in the <Foo> component',
             ),
@@ -291,12 +289,6 @@ describe('ReactDOMConsoleErrorReporting', () => {
               message: 'Boom',
             }),
           ],
-          [
-            // TODO: This is duplicated only with createRoot. Why?
-            expect.objectContaining({
-              message: 'Boom',
-            }),
-          ],
         ]);
       }
 

filename: packages/react-reconciler/src/ReactFiberThrow.new.js
patch: @@ -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;
 }

filename: packages/react-reconciler/src/ReactFiberThrow.old.js
patch: @@ -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;
 }