From d1a3291df20e42cf61550bd00a86ad1e526dd0cc Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 23 Jun 2021 20:04:44 +0100 Subject: [PATCH] Move error logging to update callback This prevents double logging for gDSFE boundaries with createRoot. --- .../ReactDOMConsoleErrorReporting-test.js | 14 +------------- .../react-reconciler/src/ReactFiberThrow.new.js | 15 +++++++-------- .../react-reconciler/src/ReactFiberThrow.old.js | 15 +++++++-------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index bbaffe5489b52..7b36531ee130d 100644 --- 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 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 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 index e100561ed86e5..398ae98abef60 100644 --- 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,6 +124,7 @@ 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. @@ -126,9 +132,6 @@ function createClassErrorUpdate( // 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 index cdb02cb8f1886..42e2557938579 100644 --- 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,6 +124,7 @@ 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. @@ -126,9 +132,6 @@ function createClassErrorUpdate( // 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; }