From 9bd771a727892ec0a933770ea5d237d5d800ddc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Mar 2024 17:17:07 -0700 Subject: [PATCH] Remove invokeGuardedCallback and replay trick (#28515) We broke the ability to "break on uncaught exceptions" by adding a try/catch higher up in the scheduling. We're giving up on fixing that so we can remove the replay trick inside an event handler. The issue with that approach is that we end up double logging a lot of errors in DEV since they get reported to the page. It's also a lot of complexity around this feature. --- fixtures/dom/src/toWarnDev.js | 5 - .../__tests__/ReactCacheOld-test.internal.js | 4 - .../src/__tests__/TimelineProfiler-test.js | 4 - .../src/events/DOMPluginEventSystem.js | 26 +- .../__tests__/InvalidEventListeners-test.js | 22 +- .../ReactBrowserEventEmitter-test.js | 10 +- .../__tests__/ReactCompositeComponent-test.js | 15 - .../ReactDOMConsoleErrorReporting-test.js | 253 ++++--------- ...eactDOMConsoleErrorReportingLegacy-test.js | 159 +++----- .../src/__tests__/ReactDOMFizzServer-test.js | 32 +- .../__tests__/ReactDOMHydrationDiff-test.js | 9 - ...tDOMServerIntegrationLegacyContext-test.js | 2 +- ...tionLegacyContextDisabled-test.internal.js | 2 +- .../ReactDOMServerIntegrationSelect-test.js | 4 +- ...DOMServerPartialHydration-test.internal.js | 5 +- .../ReactErrorBoundaries-test.internal.js | 5 +- ...eactLegacyErrorBoundaries-test.internal.js | 5 +- .../src/test-utils/ReactTestUtils.js | 24 +- .../src/legacy-events/EventBatching.js | 4 +- .../src/legacy-events/EventPluginUtils.js | 25 +- packages/react-reconciler/src/ReactFiber.js | 54 --- .../src/ReactFiberCommitWork.js | 15 - .../src/ReactFiberErrorLogger.js | 39 +- .../src/ReactFiberHydrationContext.js | 7 - .../src/ReactFiberWorkLoop.js | 99 +---- ...rorBoundaryReconciliation-test.internal.js | 4 - .../__tests__/ReactBatching-test.internal.js | 4 - .../ReactConcurrentErrorRecovery-test.js | 70 +--- .../src/__tests__/ReactHooks-test.internal.js | 7 - ...tIncrementalErrorHandling-test.internal.js | 8 +- .../ReactIncrementalErrorLogging-test.js | 175 +++++---- ...actIncrementalErrorReplay-test.internal.js | 43 --- .../src/__tests__/ReactLazy-test.internal.js | 4 - .../__tests__/ReactSuspense-test.internal.js | 4 - .../ReactSuspenseEffectsSemantics-test.js | 9 +- .../ReactSuspenseFuzz-test.internal.js | 4 - .../ReactSuspensePlaceholder-test.internal.js | 1 - .../ReactSuspenseWithNoopRenderer-test.js | 4 - .../ReactTestRenderer-test.internal.js | 1 - .../ReactCoffeeScriptClass-test.coffee | 4 +- .../react/src/__tests__/ReactES6Class-test.js | 6 +- .../ReactElementValidator-test.internal.js | 4 - .../__tests__/ReactProfiler-test.internal.js | 348 ++++++++---------- .../__tests__/ReactTypeScriptClass-test.ts | 6 +- .../src/__tests__/forwardRef-test.internal.js | 3 - packages/shared/ReactErrorUtils.js | 125 ------- packages/shared/ReactFeatureFlags.js | 4 - .../ReactErrorUtils-test.internal.js | 210 ----------- .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.native.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../forks/ReactFeatureFlags.www-dynamic.js | 4 - .../shared/forks/ReactFeatureFlags.www.js | 1 - .../forks/invokeGuardedCallbackImpl.www.js | 34 -- packages/shared/invokeGuardedCallbackImpl.js | 215 ----------- .../useSyncExternalStoreShared-test.js | 23 +- scripts/jest/shouldIgnoreConsoleError.js | 26 +- scripts/rollup/forks.js | 12 - 60 files changed, 512 insertions(+), 1685 deletions(-) delete mode 100644 packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js delete mode 100644 packages/shared/ReactErrorUtils.js delete mode 100644 packages/shared/__tests__/ReactErrorUtils-test.internal.js delete mode 100644 packages/shared/forks/invokeGuardedCallbackImpl.www.js delete mode 100644 packages/shared/invokeGuardedCallbackImpl.js diff --git a/fixtures/dom/src/toWarnDev.js b/fixtures/dom/src/toWarnDev.js index 13b3d928111cb..6e275241e98fa 100644 --- a/fixtures/dom/src/toWarnDev.js +++ b/fixtures/dom/src/toWarnDev.js @@ -7,11 +7,6 @@ const util = require('util'); function shouldIgnoreConsoleError(format, args) { if (__DEV__) { if (typeof format === 'string') { - if (format.indexOf('Error: Uncaught [') === 0) { - // This looks like an uncaught error from invokeGuardedCallback() wrapper - // in development that is reported by jsdom. Ignore because it's noisy. - return true; - } if (format.indexOf('The above error occurred') === 0) { // This looks like an error addendum from ReactFiberErrorLogger. // Ignore it too. diff --git a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js index f114d30b87201..08318d5f512fe 100644 --- a/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js +++ b/packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js @@ -12,7 +12,6 @@ let ReactCache; let createResource; let React; -let ReactFeatureFlags; let ReactNoop; let Scheduler; let Suspense; @@ -27,9 +26,6 @@ describe('ReactCache', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); Suspense = React.Suspense; ReactCache = require('react-cache'); diff --git a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js index b136be6110218..a09b4e54de4ca 100644 --- a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js +++ b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js @@ -870,7 +870,6 @@ describe('Timeline profiler', () => { "--component-render-start-ErrorBoundary", "--component-render-stop", "--component-render-start-ExampleThatThrows", - "--component-render-start-ExampleThatThrows", "--component-render-stop", "--error-ExampleThatThrows-mount-Expected error", "--render-stop", @@ -878,7 +877,6 @@ describe('Timeline profiler', () => { "--component-render-start-ErrorBoundary", "--component-render-stop", "--component-render-start-ExampleThatThrows", - "--component-render-start-ExampleThatThrows", "--component-render-stop", "--error-ExampleThatThrows-mount-Expected error", "--render-stop", @@ -2161,10 +2159,8 @@ describe('Timeline profiler', () => { await waitForAll([ 'ErrorBoundary render', 'ExampleThatThrows', - 'ExampleThatThrows', 'ErrorBoundary render', 'ExampleThatThrows', - 'ExampleThatThrows', 'ErrorBoundary fallback', ]); diff --git a/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js b/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js index ca89e1d1bd8ab..5bb1fdc91150e 100644 --- a/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js @@ -55,10 +55,6 @@ import { enableFloat, enableFormActions, } from 'shared/ReactFeatureFlags'; -import { - invokeGuardedCallbackAndCatchFirstError, - rethrowCaughtError, -} from 'shared/ReactErrorUtils'; import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener'; import { removeEventListener, @@ -234,14 +230,25 @@ export const nonDelegatedEvents: Set = new Set([ ...mediaEventTypes, ]); +let hasError: boolean = false; +let caughtError: mixed = null; + function executeDispatch( event: ReactSyntheticEvent, listener: Function, currentTarget: EventTarget, ): void { - const type = event.type || 'unknown-event'; event.currentTarget = currentTarget; - invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event); + try { + listener(event); + } catch (error) { + if (!hasError) { + hasError = true; + caughtError = error; + } else { + // TODO: Make sure this error gets logged somehow. + } + } event.currentTarget = null; } @@ -283,7 +290,12 @@ export function processDispatchQueue( // event system doesn't use pooling. } // This would be a good time to rethrow if any of the event handlers threw. - rethrowCaughtError(); + if (hasError) { + const error = caughtError; + hasError = false; + caughtError = null; + throw error; + } } function dispatchEventsForPlugins( diff --git a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js index e7c9b5f9610ca..e412256ab2678 100644 --- a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js +++ b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js @@ -43,7 +43,7 @@ describe('InvalidEventListeners', () => { ); const node = container.firstChild; - spyOnProd(console, 'error'); + console.error = jest.fn(); const uncaughtErrors = []; function handleWindowError(e) { @@ -70,18 +70,16 @@ describe('InvalidEventListeners', () => { }), ); - if (!__DEV__) { - expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error.mock.calls[0][0]).toEqual( - expect.objectContaining({ - detail: expect.objectContaining({ - message: - 'Expected `onClick` listener to be a function, instead got a value of `string` type.', - }), - type: 'unhandled exception', + expect(console.error).toHaveBeenCalledTimes(1); + expect(console.error.mock.calls[0][0]).toEqual( + expect.objectContaining({ + detail: expect.objectContaining({ + message: + 'Expected `onClick` listener to be a function, instead got a value of `string` type.', }), - ); - } + type: 'unhandled exception', + }), + ); }); it('should not prevent null listeners, at dispatch', async () => { diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index 5e27986c766a0..9c023f06d9d2a 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -202,21 +202,13 @@ describe('ReactBrowserEventEmitter', () => { expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); expect(idCallOrder[2]).toBe(GRANDPARENT); - expect(errorHandler).toHaveBeenCalledTimes(__DEV__ ? 2 : 1); + expect(errorHandler).toHaveBeenCalledTimes(1); expect(errorHandler.mock.calls[0][0]).toEqual( expect.objectContaining({ error: expect.any(Error), message: 'Handler interrupted', }), ); - if (__DEV__) { - expect(errorHandler.mock.calls[1][0]).toEqual( - expect.objectContaining({ - error: expect.any(Error), - message: 'Handler interrupted', - }), - ); - } } finally { window.removeEventListener('error', errorHandler); } diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 42d7b3133d36e..bf082369e0610 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -1231,13 +1231,6 @@ describe('ReactCompositeComponent', () => { }); }).toThrow(); }).toErrorDev([ - // Expect two errors because invokeGuardedCallback will dispatch an error event, - // Causing the warning to be logged again. - 'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' + - 'did you accidentally return an object from the constructor?', - 'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' + - 'did you accidentally return an object from the constructor?', - // And then two more because we retry errors. 'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' + 'did you accidentally return an object from the constructor?', 'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' + @@ -1278,14 +1271,6 @@ describe('ReactCompositeComponent', () => { }); }).toThrow(); }).toErrorDev([ - // Expect two errors because invokeGuardedCallback will dispatch an error event, - // Causing the warning to be logged again. - 'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' + - 'you may have forgotten to define `render`.', - 'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' + - 'you may have forgotten to define `render`.', - - // And then two more because we retry errors. 'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' + 'you may have forgotten to define `render`.', 'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' + diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index ad5826fe393ea..bd80905478bf0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -56,7 +56,8 @@ describe('ReactDOMConsoleErrorReporting', () => { describe('ReactDOMClient.createRoot', () => { it('logs errors during event handlers', async () => { - spyOnDevAndProd(console, 'error'); + const originalError = console.error; + console.error = jest.fn(); function Foo() { return ( @@ -82,76 +83,34 @@ describe('ReactDOMConsoleErrorReporting', () => { ); }); - if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported because we're in a browser click event: - expect.objectContaining({ - message: 'Boom', - }), - ], - [ - // This one is jsdom-only. Real browser deduplicates it. - // (In DEV, we have a nested event due to guarded callback.) - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); - expect(console.error.mock.calls).toEqual([ - [ - // Reported because we're in a browser click event: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], - [ - // This one is jsdom-only. Real browser deduplicates it. - // (In DEV, we have a nested event due to guarded callback.) - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], - ]); - } else { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported because we're in a browser click event: - expect.objectContaining({ + expect(windowOnError.mock.calls).toEqual([ + [ + // Reported because we're in a browser click event: + expect.objectContaining({ + message: 'Boom', + }), + ], + ]); + expect(console.error.mock.calls).toEqual([ + [ + // Reported because we're in a browser click event: + expect.objectContaining({ + detail: expect.objectContaining({ message: 'Boom', }), - ], - ]); - expect(console.error.mock.calls).toEqual([ - [ - // Reported because we're in a browser click event: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], - ]); - } + type: 'unhandled exception', + }), + ], + ]); // Check next render doesn't throw. windowOnError.mockReset(); - console.error.mockReset(); + console.error = originalError; await act(() => { root.render(); }); expect(container.textContent).toBe('OK'); expect(windowOnError.mock.calls).toEqual([]); - if (__DEV__) { - expect(console.error.mock.calls).toEqual([]); - } }); it('logs render errors without an error boundary', async () => { @@ -168,50 +127,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - [ - // This is only duplicated with createRoot - // because it retries once with a sync render. - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [ - // Reported due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], - [ - // This is only duplicated with createRoot - // because it retries once with a sync render. - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -254,50 +187,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - [ - // This is only duplicated with createRoot - // because it retries once with a sync render. - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [ - // Reported by jsdom due to the guarded callback: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], - [ - // This is only duplicated with createRoot - // because it retries once with a sync render. + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -340,33 +247,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [ - // Reported due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -412,33 +310,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [ - // Reported by jsdom due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -481,33 +370,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [ - // Reported due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -553,33 +433,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [ - // Reported by jsdom due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js index 7514a7dab9247..e75b4b5e29045 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js @@ -57,7 +57,8 @@ describe('ReactDOMConsoleErrorReporting', () => { describe('ReactDOM.render', () => { // @gate !disableLegacyMode it('logs errors during event handlers', async () => { - spyOnDevAndProd(console, 'error'); + const originalError = console.error; + console.error = jest.fn(); function Foo() { return ( @@ -90,13 +91,6 @@ describe('ReactDOMConsoleErrorReporting', () => { message: 'Boom', }), ], - [ - // This one is jsdom-only. Real browser deduplicates it. - // (In DEV, we have a nested event due to guarded callback.) - expect.objectContaining({ - message: 'Boom', - }), - ], ]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], @@ -109,16 +103,6 @@ describe('ReactDOMConsoleErrorReporting', () => { type: 'unhandled exception', }), ], - [ - // This one is jsdom-only. Real browser deduplicates it. - // (In DEV, we have a nested event due to guarded callback.) - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], ]); } else { expect(windowOnError.mock.calls).toEqual([ @@ -155,6 +139,8 @@ describe('ReactDOMConsoleErrorReporting', () => { [expect.stringContaining('ReactDOM.render is no longer supported')], ]); } + + console.error = originalError; }); // @gate !disableLegacyMode @@ -170,34 +156,24 @@ describe('ReactDOMConsoleErrorReporting', () => { }).toThrow('Boom'); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], [ - // Reported due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -243,34 +219,25 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], [ - // Reported by jsdom due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -314,34 +281,25 @@ describe('ReactDOMConsoleErrorReporting', () => { }).toThrow('Boom'); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], [ - // Reported due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -390,34 +348,25 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], [ - // Reported by jsdom due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -462,34 +411,25 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - expect(windowOnError.mock.calls).toEqual([ - [ - // Reported due to guarded callback: - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], [ - // Reported due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('Consider adding an error boundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ @@ -538,34 +478,25 @@ describe('ReactDOMConsoleErrorReporting', () => { }); if (__DEV__) { - // Reported due to guarded callback: - expect(windowOnError.mock.calls).toEqual([ - [ - expect.objectContaining({ - message: 'Boom', - }), - ], - ]); + expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ [expect.stringContaining('ReactDOM.render is no longer supported')], [ - // Reported by jsdom due to the guarded callback: + // Formatting + expect.stringContaining('%o'), expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', + message: 'Boom', }), - ], - [ // Addendum by React: expect.stringContaining( 'The above error occurred in the component', ), + expect.stringContaining('Foo'), + expect.stringContaining('ErrorBoundary'), ], ]); } else { - // The top-level error was caught with try/catch, and there's no guarded callback, + // The top-level error was caught with try/catch, // so in production we don't see an error event. expect(windowOnError.mock.calls).toEqual([]); expect(console.error.mock.calls).toEqual([ diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 0d49e7bae122a..e6ff3e2c22089 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3234,7 +3234,7 @@ describe('ReactDOMFizzServer', () => { it('logs regular (non-hydration) errors when the UI recovers', async () => { let shouldThrow = true; - function A() { + function A({unused}) { if (shouldThrow) { Scheduler.log('Oops!'); throw new Error('Oops!'); @@ -3267,7 +3267,7 @@ describe('ReactDOMFizzServer', () => { }); // Partially render A, but yield before the render has finished - await waitFor(['Oops!', 'Oops!']); + await waitFor(['Oops!']); // React will try rendering again synchronously. During the retry, A will // not throw. This simulates a concurrent data race that is fixed by @@ -4643,8 +4643,7 @@ describe('ReactDOMFizzServer', () => { await waitForAll([]); }); - // @gate __DEV__ - it('does not invokeGuardedCallback for errors after the first hydration error', async () => { + it('does not log for errors after the first hydration error', async () => { // We can't use the toErrorDev helper here because this is async. const originalConsoleError = console.error; const mockError = jest.fn(); @@ -4717,21 +4716,13 @@ describe('ReactDOMFizzServer', () => { }); await waitForAll([ 'throwing: first error', - // this repeated first error is the invokeGuardedCallback throw - 'throwing: first error', // onRecoverableError because the UI recovered without surfacing the // error to the user. 'Logged recoverable error: first error', 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', ]); - // These Uncaught error calls are the error reported by the runtime (jsdom here, browser in actual use) - // when invokeGuardedCallback is used to replay an error in dev using event dispatching in the document - expect(mockError.mock.calls).toEqual([ - // we only get one because we suppress invokeGuardedCallback after the first one when hydrating in a - // suspense boundary - ['Error: Uncaught [Error: first error]'], - ]); + expect(mockError.mock.calls).toEqual([]); mockError.mockClear(); expect(getVisibleChildren(container)).toEqual( @@ -4749,8 +4740,7 @@ describe('ReactDOMFizzServer', () => { } }); - // @gate __DEV__ - it('does not invokeGuardedCallback for errors after a preceding fiber suspends', async () => { + it('does not log for errors after a preceding fiber suspends', async () => { // We can't use the toErrorDev helper here because this is async. const originalConsoleError = console.error; const mockError = jest.fn(); @@ -4853,7 +4843,6 @@ describe('ReactDOMFizzServer', () => { ); await unsuspend(); await waitForAll([ - 'throwing: first error', 'throwing: first error', 'Logged recoverable error: first error', 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', @@ -4870,7 +4859,6 @@ describe('ReactDOMFizzServer', () => { } }); - // @gate __DEV__ it('(outdated behavior) suspending after erroring will cause errors previously queued to be silenced until the boundary resolves', async () => { // NOTE: This test was originally written to test a scenario that doesn't happen // anymore. If something errors during hydration, we immediately unwind the @@ -4968,20 +4956,12 @@ describe('ReactDOMFizzServer', () => { }, }); await waitForAll([ - 'throwing: first error', - // duplicate because first error is re-done in invokeGuardedCallback 'throwing: first error', 'suspending', 'Logged recoverable error: first error', 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', ]); - // These Uncaught error calls are the error reported by the runtime (jsdom here, browser in actual use) - // when invokeGuardedCallback is used to replay an error in dev using event dispatching in the document - expect(mockError.mock.calls).toEqual([ - // we only get one because we suppress invokeGuardedCallback after the first one when hydrating in a - // suspense boundary - ['Error: Uncaught [Error: first error]'], - ]); + expect(mockError.mock.calls).toEqual([]); mockError.mockClear(); expect(getVisibleChildren(container)).toEqual( diff --git a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js index 76a8229e5a89f..0eb9cc3fe47d7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -51,15 +51,6 @@ describe('ReactDOMServerHydration', () => { if (format instanceof Error) { return 'Caught [' + format.message + ']'; } - if ( - format !== null && - typeof format === 'object' && - String(format).indexOf('Error: Uncaught [') === 0 - ) { - // Ignore errors captured by jsdom and their stacks. - // We only want console errors in this suite. - return null; - } rest[rest.length - 1] = normalizeCodeLocInfo(rest[rest.length - 1]); return util.format(format, ...rest); } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js index 14c3688295946..017217511fce9 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js @@ -278,7 +278,7 @@ describe('ReactDOMServerIntegration', () => { } const e = await render( , - render === clientRenderOnBadMarkup ? 4 : 1, + render === clientRenderOnBadMarkup ? 2 : 1, ); expect(e.textContent).toBe('nope'); }, diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContextDisabled-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContextDisabled-test.internal.js index d6abf530fbb40..e2af97951ea48 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContextDisabled-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContextDisabled-test.internal.js @@ -105,7 +105,7 @@ describe('ReactDOMServerIntegrationLegacyContextDisabled', () => { , - render === clientRenderOnBadMarkup ? 6 : 3, + render === clientRenderOnBadMarkup ? 5 : 3, ); expect(e.textContent).toBe('{}undefinedundefined'); expect(lifecycleContextLog).toEqual([]); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js index 13c7f78ea831e..8114092a42b0d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js @@ -254,7 +254,7 @@ describe('ReactDOMServerIntegrationSelect', () => { , - 2, + 1, ); expect(e.firstChild.selected).toBe(false); expect(e.lastChild.selected).toBe(true); @@ -269,7 +269,7 @@ describe('ReactDOMServerIntegrationSelect', () => { , - 2, + 1, ); expect(e.firstChild.selected).toBe(true); expect(e.lastChild.selected).toBe(false); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index d9cfd7e5eee1a..9ee8ee5a8130e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -349,9 +349,8 @@ describe('ReactDOMServerPartialHydration', () => { ); if (__DEV__) { - const secondToLastCall = - mockError.mock.calls[mockError.mock.calls.length - 2]; - expect(secondToLastCall).toEqual([ + const lastCall = mockError.mock.calls[mockError.mock.calls.length - 1]; + expect(lastCall).toEqual([ 'Warning: Expected server HTML to contain a matching <%s> in <%s>.%s', 'article', 'section', diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 403a3de7b6b17..284cb0599aded 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -14,7 +14,6 @@ let React; let ReactDOM; let ReactDOMClient; let act; -let ReactFeatureFlags; let Scheduler; describe('ReactErrorBoundaries', () => { @@ -42,8 +41,6 @@ describe('ReactErrorBoundaries', () => { jest.useFakeTimers(); jest.resetModules(); PropTypes = require('prop-types'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); React = require('react'); @@ -710,7 +707,7 @@ describe('ReactErrorBoundaries', () => { }); if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error.mock.calls[0][0]).toContain( + expect(console.error.mock.calls[0][2]).toContain( 'The above error occurred in the component:', ); } diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index d16d6253e36d1..bed800fb1da73 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -12,7 +12,6 @@ let PropTypes; let React; let ReactDOM; -let ReactFeatureFlags; // TODO: Refactor this test once componentDidCatch setState is deprecated. describe('ReactLegacyErrorBoundaries', () => { @@ -39,8 +38,6 @@ describe('ReactLegacyErrorBoundaries', () => { beforeEach(() => { jest.resetModules(); PropTypes = require('prop-types'); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; ReactDOM = require('react-dom'); React = require('react'); @@ -689,7 +686,7 @@ describe('ReactLegacyErrorBoundaries', () => { expect(console.error.mock.calls[0][0]).toContain( 'ReactDOM.render is no longer supported', ); - expect(console.error.mock.calls[1][0]).toContain( + expect(console.error.mock.calls[1][2]).toContain( 'The above error occurred in the component:', ); } diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index c2af208a0a6a0..8a4f2eb9ba7cd 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -21,10 +21,6 @@ import { } from 'react-reconciler/src/ReactWorkTags'; import {SyntheticEvent} from 'react-dom-bindings/src/events/SyntheticEvent'; import {ELEMENT_NODE} from 'react-dom-bindings/src/client/HTMLNodeType'; -import { - rethrowCaughtError, - invokeGuardedCallbackAndCatchFirstError, -} from 'shared/ReactErrorUtils'; import {enableFloat} from 'shared/ReactFeatureFlags'; import assign from 'shared/assign'; import isArray from 'shared/isArray'; @@ -354,6 +350,9 @@ function nativeTouchData(x, y) { // EventPropagator.js, as they deviated from ReactDOM's newer // implementations. +let hasError: boolean = false; +let caughtError: mixed = null; + /** * Dispatch the event to the listener. * @param {SyntheticEvent} event SyntheticEvent to handle @@ -361,9 +360,15 @@ function nativeTouchData(x, y) { * @param {*} inst Internal component instance */ function executeDispatch(event, listener, inst) { - const type = event.type || 'unknown-event'; event.currentTarget = getNodeFromInstance(inst); - invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event); + try { + listener(event); + } catch (error) { + if (!hasError) { + hasError = true; + caughtError = error; + } + } event.currentTarget = null; } @@ -619,7 +624,12 @@ function makeSimulator(eventType) { // do that since we're by-passing it here. enqueueStateRestore(domNode); executeDispatchesAndRelease(event); - rethrowCaughtError(); + if (hasError) { + const error = caughtError; + hasError = false; + caughtError = null; + throw error; + } }); restoreStateIfNeeded(); }; diff --git a/packages/react-native-renderer/src/legacy-events/EventBatching.js b/packages/react-native-renderer/src/legacy-events/EventBatching.js index cad3671723ed7..4a65042fc2e7b 100644 --- a/packages/react-native-renderer/src/legacy-events/EventBatching.js +++ b/packages/react-native-renderer/src/legacy-events/EventBatching.js @@ -6,12 +6,10 @@ * @flow */ -import {rethrowCaughtError} from 'shared/ReactErrorUtils'; - import type {ReactSyntheticEvent} from './ReactSyntheticEventType'; import accumulateInto from './accumulateInto'; import forEachAccumulated from './forEachAccumulated'; -import {executeDispatchesInOrder} from './EventPluginUtils'; +import {executeDispatchesInOrder, rethrowCaughtError} from './EventPluginUtils'; /** * Internal queue of events that have accumulated their dispatches and are diff --git a/packages/react-native-renderer/src/legacy-events/EventPluginUtils.js b/packages/react-native-renderer/src/legacy-events/EventPluginUtils.js index d61bd71ee0ea8..f97d7b5488b50 100644 --- a/packages/react-native-renderer/src/legacy-events/EventPluginUtils.js +++ b/packages/react-native-renderer/src/legacy-events/EventPluginUtils.js @@ -5,9 +5,11 @@ * LICENSE file in the root directory of this source tree. */ -import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils'; import isArray from 'shared/isArray'; +let hasError = false; +let caughtError = null; + export let getFiberCurrentPropsFromNode = null; export let getInstanceFromNode = null; export let getNodeFromInstance = null; @@ -62,9 +64,17 @@ function validateEventDispatches(event) { * @param {*} inst Internal component instance */ export function executeDispatch(event, listener, inst) { - const type = event.type || 'unknown-event'; event.currentTarget = getNodeFromInstance(inst); - invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event); + try { + listener(event); + } catch (error) { + if (!hasError) { + hasError = true; + caughtError = error; + } else { + // TODO: Make sure this error gets logged somehow. + } + } event.currentTarget = null; } @@ -170,3 +180,12 @@ export function executeDirectDispatch(event) { export function hasDispatches(event) { return !!event._dispatchListeners; } + +export function rethrowCaughtError() { + if (hasError) { + const error = caughtError; + hasError = false; + caughtError = null; + throw error; + } +} diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 982e8b5905456..786761ebb4daa 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -879,57 +879,3 @@ export function createFiberFromPortal( }; return fiber; } - -// Used for stashing WIP properties to replay failed work in DEV. -export function assignFiberPropertiesInDEV( - target: Fiber | null, - source: Fiber, -): Fiber { - if (target === null) { - // This Fiber's initial properties will always be overwritten. - // We only use a Fiber to ensure the same hidden class so DEV isn't slow. - target = createFiber(IndeterminateComponent, null, null, NoMode); - } - - // This is intentionally written as a list of all properties. - // We tried to use Object.assign() instead but this is called in - // the hottest path, and Object.assign() was too slow: - // https://github.com/facebook/react/issues/12502 - // This code is DEV-only so size is not a concern. - - target.tag = source.tag; - target.key = source.key; - target.elementType = source.elementType; - target.type = source.type; - target.stateNode = source.stateNode; - target.return = source.return; - target.child = source.child; - target.sibling = source.sibling; - target.index = source.index; - target.ref = source.ref; - target.refCleanup = source.refCleanup; - target.pendingProps = source.pendingProps; - target.memoizedProps = source.memoizedProps; - target.updateQueue = source.updateQueue; - target.memoizedState = source.memoizedState; - target.dependencies = source.dependencies; - target.mode = source.mode; - target.flags = source.flags; - target.subtreeFlags = source.subtreeFlags; - target.deletions = source.deletions; - target.lanes = source.lanes; - target.childLanes = source.childLanes; - target.alternate = source.alternate; - if (enableProfilerTimer) { - target.actualDuration = source.actualDuration; - target.actualStartTime = source.actualStartTime; - target.selfBaseDuration = source.selfBaseDuration; - target.treeBaseDuration = source.treeBaseDuration; - } - - target._debugInfo = source._debugInfo; - target._debugOwner = source._debugOwner; - target._debugNeedsRemount = source._debugNeedsRemount; - target._debugHookTypes = source._debugHookTypes; - return target; -} diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index e3992a6343080..fdd289d93280f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -190,7 +190,6 @@ import { } from './ReactHookEffectTags'; import {didWarnAboutReassigningProps} from './ReactFiberBeginWork'; import {doesFiberContain} from './ReactFiberTreeReflection'; -import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils'; import { isDevToolsPresent, markComponentPassiveEffectMountStarted, @@ -244,20 +243,6 @@ function shouldProfile(current: Fiber): boolean { ); } -export function reportUncaughtErrorInDEV(error: mixed) { - // Wrapping each small part of the commit phase into a guarded - // callback is a bit too slow (https://github.com/facebook/react/pull/21666). - // But we rely on it to surface errors to DEV tools like overlays - // (https://github.com/facebook/react/issues/21712). - // As a compromise, rethrow only caught errors in a guard. - if (__DEV__) { - invokeGuardedCallback(null, () => { - throw error; - }); - clearCaughtError(); - } -} - function callComponentWillUnmountWithTimer(current: Fiber, instance: any) { instance.props = current.memoizedProps; instance.state = current.memoizedState; diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index 15a91f236de40..14a1ebb352f89 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -11,7 +11,6 @@ import type {Fiber} from './ReactInternalTypes'; import type {CapturedValue} from './ReactCapturedValue'; import {showErrorDialog} from './ReactFiberErrorDialog'; -import {ClassComponent} from './ReactWorkTags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import {HostRoot} from 'react-reconciler/src/ReactWorkTags'; @@ -33,24 +32,8 @@ export function logCapturedError( const source = errorInfo.source; const stack = errorInfo.stack; const componentStack = stack !== null ? stack : ''; - // Browsers support silencing uncaught errors by calling - // `preventDefault()` in window `error` handler. - // We record this information as an expando on the error. - if (error != null && error._suppressLogging) { - if (boundary.tag === ClassComponent) { - // The error is recoverable and was silenced. - // Ignore it and don't print the stack addendum. - // This is handy for testing error boundaries without noise. - return; - } - // The error is fatal. Since the silencing might have - // been accidental, we'll surface it anyway. - // However, the browser would have silenced the original error - // so we'll print it first, and then print the stack addendum. - console['error'](error); // Don't transform to our wrapper - // For a more detailed description of this block, see: - // https://github.com/facebook/react/pull/13384 - } + // TODO: There's no longer a way to silence these warnings e.g. for tests. + // See https://github.com/facebook/react/pull/13384 const componentName = source ? getComponentNameFromFiber(source) : null; const componentNameMessage = componentName @@ -69,15 +52,17 @@ export function logCapturedError( `React will try to recreate this component tree from scratch ` + `using the error boundary you provided, ${errorBoundaryName}.`; } - const combinedMessage = - `${componentNameMessage}\n${componentStack}\n\n` + - `${errorBoundaryMessage}`; - // In development, we provide our own message with just the component stack. - // We don't include the original error message and JS stack because the browser - // has already printed it. Even if the application swallows the error, it is still - // displayed by the browser thanks to the DEV-only fake event trick in ReactErrorUtils. - console['error'](combinedMessage); // Don't transform to our wrapper + // In development, we provide our own message which includes the component stack + // in addition to the error. + console['error']( + // Don't transform to our wrapper + '%o\n\n%s\n%s\n\n%s', + error, + componentNameMessage, + componentStack, + errorBoundaryMessage, + ); } else { // In production, we print the error directly. // This will include the message, the JS stack, and anything the browser wants to show. diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index 4d59710c758e2..90d87e13d7b33 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -116,13 +116,6 @@ export function markDidThrowWhileHydratingDEV() { } } -export function didSuspendOrErrorWhileHydratingDEV(): boolean { - if (__DEV__) { - return didSuspendOrErrorDEV; - } - return false; -} - function enterHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 74b276bfffd69..152b0b460b2f4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -25,7 +25,6 @@ import type {OffscreenInstance} from './ReactFiberActivityComponent'; import type {RenderTaskFn} from './ReactFiberRootScheduler'; import { - replayFailedUnitOfWorkWithInvokeGuardedCallback, enableCreateEventHandleAPI, enableProfilerTimer, enableProfilerCommitHooks, @@ -77,16 +76,9 @@ import { preloadInstance, } from './ReactFiberConfig'; -import { - createWorkInProgress, - assignFiberPropertiesInDEV, - resetWorkInProgress, -} from './ReactFiber'; +import {createWorkInProgress, resetWorkInProgress} from './ReactFiber'; import {isRootDehydrated} from './ReactFiberShellHydration'; -import { - getIsHydrating, - didSuspendOrErrorWhileHydratingDEV, -} from './ReactFiberHydrationContext'; +import {getIsHydrating} from './ReactFiberHydrationContext'; import { NoMode, ProfileMode, @@ -173,7 +165,7 @@ import { import {requestCurrentTransition} from './ReactFiberTransition'; import { SelectiveHydrationException, - beginWork as originalBeginWork, + beginWork, replayFunctionComponent, } from './ReactFiberBeginWork'; import {completeWork} from './ReactFiberCompleteWork'; @@ -194,7 +186,6 @@ import { reconnectPassiveEffects, reappearLayoutEffects, disconnectPassiveEffect, - reportUncaughtErrorInDEV, invokeLayoutEffectMountInDEV, invokePassiveEffectMountInDEV, invokeLayoutEffectUnmountInDEV, @@ -237,11 +228,6 @@ import { resetCurrentFiber as resetCurrentDebugFiberInDEV, setCurrentFiber as setCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; -import { - invokeGuardedCallback, - hasCaughtError, - clearCaughtError, -} from 'shared/ReactErrorUtils'; import { isDevToolsPresent, markCommitStarted, @@ -3397,7 +3383,6 @@ export function captureCommitPhaseError( error: mixed, ) { if (__DEV__) { - reportUncaughtErrorInDEV(error); setIsRunningInsertionEffect(false); } if (sourceFiber.tag === HostRoot) { @@ -3884,84 +3869,6 @@ export function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber: Fiber) { } } -let beginWork; -if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { - const dummyFiber = null; - beginWork = (current: null | Fiber, unitOfWork: Fiber, lanes: Lanes) => { - // If a component throws an error, we replay it again in a synchronously - // dispatched event, so that the debugger will treat it as an uncaught - // error See ReactErrorUtils for more information. - - // Before entering the begin phase, copy the work-in-progress onto a dummy - // fiber. If beginWork throws, we'll use this to reset the state. - const originalWorkInProgressCopy = assignFiberPropertiesInDEV( - dummyFiber, - unitOfWork, - ); - try { - return originalBeginWork(current, unitOfWork, lanes); - } catch (originalError) { - if ( - didSuspendOrErrorWhileHydratingDEV() || - originalError === SuspenseException || - originalError === SelectiveHydrationException || - (originalError !== null && - typeof originalError === 'object' && - typeof originalError.then === 'function') - ) { - // Don't replay promises. - // Don't replay errors if we are hydrating and have already suspended or handled an error - throw originalError; - } - - // Don't reset current debug fiber, since we're about to work on the - // same fiber again. - - // Unwind the failed stack frame - resetSuspendedWorkLoopOnUnwind(unitOfWork); - unwindInterruptedWork(current, unitOfWork, workInProgressRootRenderLanes); - - // Restore the original properties of the fiber. - assignFiberPropertiesInDEV(unitOfWork, originalWorkInProgressCopy); - - if (enableProfilerTimer && unitOfWork.mode & ProfileMode) { - // Reset the profiler timer. - startProfilerTimer(unitOfWork); - } - - // Run beginWork again. - invokeGuardedCallback( - null, - originalBeginWork, - null, - current, - unitOfWork, - lanes, - ); - - if (hasCaughtError()) { - const replayError = clearCaughtError(); - if ( - typeof replayError === 'object' && - replayError !== null && - replayError._suppressLogging && - typeof originalError === 'object' && - originalError !== null && - !originalError._suppressLogging - ) { - // If suppressed, let the flag carry over to the original error which is the one we'll rethrow. - originalError._suppressLogging = true; - } - } - // We always throw the original error in case the second render pass is not idempotent. - // This can happen if a memoized function or CommonJS module doesn't throw after first invocation. - throw originalError; - } - }; -} else { - beginWork = originalBeginWork; -} - let didWarnAboutUpdateInRender = false; let didWarnAboutUpdateInRenderForAnotherComponent; if (__DEV__) { diff --git a/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js b/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js index 0ce51699849b7..057a8de13cb6c 100644 --- a/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ErrorBoundaryReconciliation-test.internal.js @@ -3,7 +3,6 @@ describe('ErrorBoundaryReconciliation', () => { let DidCatchErrorBoundary; let GetDerivedErrorBoundary; let React; - let ReactFeatureFlags; let ReactTestRenderer; let span; let act; @@ -11,9 +10,6 @@ describe('ErrorBoundaryReconciliation', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; ReactTestRenderer = require('react-test-renderer'); React = require('react'); act = require('internal-test-utils').act; diff --git a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js index 387b6a4e5523a..4d2e8f516c01e 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js @@ -1,5 +1,4 @@ let React; -let ReactFeatureFlags; let ReactNoop; let Scheduler; let waitForAll; @@ -12,9 +11,6 @@ let act; describe('ReactBlockingMode', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); diff --git a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js index 8c4f0d3415cec..d665530082c1f 100644 --- a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js +++ b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js @@ -220,20 +220,7 @@ describe('ReactConcurrentErrorRecovery', () => { // Because we're still suspended on A, we can't show an error boundary. We // should wait for A to resolve. - if (gate(flags => flags.replayFailedUnitOfWorkWithInvokeGuardedCallback)) { - assertLog([ - 'Suspend! [A2]', - 'Loading...', - - 'Error! [B2]', - // This extra log happens when we replay the error - // in invokeGuardedCallback - 'Error! [B2]', - 'Oops!', - ]); - } else { - assertLog(['Suspend! [A2]', 'Loading...', 'Error! [B2]', 'Oops!']); - } + assertLog(['Suspend! [A2]', 'Loading...', 'Error! [B2]', 'Oops!']); // Remain on previous screen. expect(root).toMatchRenderedOutput('A1B1'); @@ -241,25 +228,7 @@ describe('ReactConcurrentErrorRecovery', () => { await act(() => { resolveText('A2'); }); - if (gate(flags => flags.replayFailedUnitOfWorkWithInvokeGuardedCallback)) { - assertLog([ - 'A2', - 'Error! [B2]', - // This extra log happens when we replay the error - // in invokeGuardedCallback - 'Error! [B2]', - 'Oops!', - - 'A2', - 'Error! [B2]', - // This extra log happens when we replay the error - // in invokeGuardedCallback - 'Error! [B2]', - 'Oops!', - ]); - } else { - assertLog(['A2', 'Error! [B2]', 'Oops!', 'A2', 'Error! [B2]', 'Oops!']); - } + assertLog(['A2', 'Error! [B2]', 'Oops!', 'A2', 'Error! [B2]', 'Oops!']); // Now we can show the error boundary that's wrapped around B. expect(root).toMatchRenderedOutput('A2Oops!'); }); @@ -323,20 +292,7 @@ describe('ReactConcurrentErrorRecovery', () => { // Because we're still suspended on B, we can't show an error boundary. We // should wait for B to resolve. - if (gate(flags => flags.replayFailedUnitOfWorkWithInvokeGuardedCallback)) { - assertLog([ - 'Error! [A2]', - // This extra log happens when we replay the error - // in invokeGuardedCallback - 'Error! [A2]', - 'Oops!', - - 'Suspend! [B2]', - 'Loading...', - ]); - } else { - assertLog(['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...']); - } + assertLog(['Error! [A2]', 'Oops!', 'Suspend! [B2]', 'Loading...']); // Remain on previous screen. expect(root).toMatchRenderedOutput('A1B1'); @@ -344,25 +300,7 @@ describe('ReactConcurrentErrorRecovery', () => { await act(() => { resolveText('B2'); }); - if (gate(flags => flags.replayFailedUnitOfWorkWithInvokeGuardedCallback)) { - assertLog([ - 'Error! [A2]', - // This extra log happens when we replay the error - // in invokeGuardedCallback - 'Error! [A2]', - 'Oops!', - 'B2', - - 'Error! [A2]', - // This extra log happens when we replay the error - // in invokeGuardedCallback - 'Error! [A2]', - 'Oops!', - 'B2', - ]); - } else { - assertLog(['Error! [A2]', 'Oops!', 'B2', 'Error! [A2]', 'Oops!', 'B2']); - } + assertLog(['Error! [A2]', 'Oops!', 'B2', 'Error! [A2]', 'Oops!', 'B2']); // Now we can show the error boundary that's wrapped around B. expect(root).toMatchRenderedOutput('Oops!B2'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index a23b904d42c7c..aeff063948915 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1042,7 +1042,6 @@ describe('ReactHooks', () => { 'Update hook called on initial render. This is likely a bug in React. Please file an issue.', ); }).toErrorDev([ - 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', 'Warning: React has detected a change in the order of Hooks called by App. ' + 'This will lead to bugs and errors if not fixed. For more information, ' + @@ -1105,9 +1104,6 @@ describe('ReactHooks', () => { , ); }).toErrorDev([ - // We see it twice due to replay - 'Context can only be read while React is rendering', - 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', 'Context can only be read while React is rendering', 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', ]); @@ -1143,9 +1139,6 @@ describe('ReactHooks', () => { , ); }).toErrorDev([ - // We see it twice due to replay - 'Context can only be read while React is rendering', - 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', 'Context can only be read while React is rendering', 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', ]); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index d60e0b5426401..1613f1e048ec7 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -10,7 +10,6 @@ 'use strict'; -let ReactFeatureFlags = require('shared/ReactFeatureFlags'); let PropTypes; let React; let ReactNoop; @@ -24,8 +23,6 @@ let waitForThrow; describe('ReactIncrementalErrorHandling', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; PropTypes = require('prop-types'); React = require('react'); ReactNoop = require('react-noop-renderer'); @@ -1513,7 +1510,8 @@ describe('ReactIncrementalErrorHandling', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error.mock.calls[0][0]).toContain( + expect(console.error.mock.calls[0][1]).toBe(notAnError); + expect(console.error.mock.calls[0][2]).toContain( 'The above error occurred in the component:', ); } else { @@ -1914,7 +1912,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(console.error.mock.calls[0][0]).toContain( 'Cannot update a component (`%s`) while rendering a different component', ); - expect(console.error.mock.calls[1][0]).toContain( + expect(console.error.mock.calls[1][2]).toContain( 'The above error occurred in the component', ); } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index e56d0f30d7bce..82edcf2db4503 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -60,22 +60,34 @@ describe('ReactIncrementalErrorLogging', () => { ); await waitForThrow('constructor error'); expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)\n\n' + - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior\\.', - ), - ) - : expect.objectContaining({ - message: 'constructor error', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'constructor error', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', + ), + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', + ), + ); + } else { + expect(console.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'constructor error', + }), + ); + } }); it('should log errors that occur during the commit phase', async () => { @@ -96,22 +108,34 @@ describe('ReactIncrementalErrorLogging', () => { ); await waitForThrow('componentDidMount error'); expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)\n\n' + - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior\\.', - ), - ) - : expect.objectContaining({ - message: 'componentDidMount error', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'componentDidMount error', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', + ), + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', + ), + ); + } else { + expect(console.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'componentDidMount error', + }), + ); + } }); it('should ignore errors thrown in log method to prevent cycle', async () => { @@ -135,22 +159,34 @@ describe('ReactIncrementalErrorLogging', () => { ); await waitForThrow('render error'); expect(logCapturedErrorCalls.length).toBe(1); - expect(logCapturedErrorCalls[0]).toEqual( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)\n\n' + - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior\\.', - ), - ) - : expect.objectContaining({ - message: 'render error', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'render error', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', + ), + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', + ), + ); + } else { + expect(logCapturedErrorCalls[0]).toEqual( + expect.objectContaining({ + message: 'render error', + }), + ); + } // The error thrown in logCapturedError should be rethrown with a clean stack expect(() => { jest.runAllTimers(); @@ -194,31 +230,40 @@ describe('ReactIncrementalErrorLogging', () => { 'render: 0', 'render: 1', - __DEV__ && 'render: 1', // replay due to invokeGuardedCallback // Retry one more time before handling error 'render: 1', - __DEV__ && 'render: 1', // replay due to invokeGuardedCallback 'componentWillUnmount: 0', ].filter(Boolean), ); expect(console.error).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( - __DEV__ - ? expect.stringMatching( - new RegExp( - 'The above error occurred in the component:\n' + - '\\s+(in|at) Foo (.*)\n' + - '\\s+(in|at) ErrorBoundary (.*)\n\n' + - 'React will try to recreate this component tree from scratch ' + - 'using the error boundary you provided, ErrorBoundary.', - ), - ) - : expect.objectContaining({ - message: 'oops', - }), - ); + if (__DEV__) { + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('%o'), + expect.objectContaining({ + message: 'oops', + }), + expect.stringContaining( + 'The above error occurred in the component:', + ), + expect.stringMatching( + new RegExp( + '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)', + ), + ), + expect.stringContaining( + 'React will try to recreate this component tree from scratch ' + + 'using the error boundary you provided, ErrorBoundary.', + ), + ); + } else { + expect(console.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'oops', + }), + ); + } }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js deleted file mode 100644 index 9b4c2462fad79..0000000000000 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js +++ /dev/null @@ -1,43 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @jest-environment node - */ - -'use strict'; - -describe('ReactIncrementalErrorReplay-test', () => { - const React = require('react'); - const ReactTestRenderer = require('react-test-renderer'); - - it('copies all keys when stashing potentially failing work', () => { - // Note: this test is fragile and relies on internals. - // We almost always try to avoid such tests, but here the cost of - // the list getting out of sync (and causing subtle bugs in rare cases) - // is higher than the cost of maintaining the test. - - // This is the method we're going to test. - // If this is no longer used, you can delete this test file.; - const {assignFiberPropertiesInDEV} = require('../ReactFiber'); - - // Get a real fiber. - const realFiber = ReactTestRenderer.create(
).root._currentFiber(); - const stash = assignFiberPropertiesInDEV(null, realFiber); - - // Verify we get all the same fields. - expect(realFiber).toEqual(stash); - - // Mutate the original. - for (const key in realFiber) { - realFiber[key] = key + '_' + Math.random(); - } - expect(realFiber).not.toEqual(stash); - - // Verify we can still "revert" to the stashed properties. - expect(assignFiberPropertiesInDEV(realFiber, stash)).toBe(realFiber); - expect(realFiber).toEqual(stash); - }); -}); diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 082d922acfb3f..27a013fa9382e 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -1,7 +1,6 @@ let React; let ReactTestRenderer; let Scheduler; -let ReactFeatureFlags; let Suspense; let lazy; let waitFor; @@ -24,9 +23,6 @@ function normalizeCodeLocInfo(str) { describe('ReactLazy', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); Suspense = React.Suspense; lazy = React.lazy; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 32ed2c1b3e34e..6c5a6c73a5711 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -1,7 +1,6 @@ let React; let ReactDOMClient; let ReactDOM; -let ReactFeatureFlags; let Scheduler; let Suspense; let act; @@ -16,9 +15,6 @@ let waitFor; describe('ReactSuspense', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js index 5200635b6fca4..19771d9757bf2 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseEffectsSemantics-test.js @@ -1980,11 +1980,10 @@ describe('ReactSuspenseEffectsSemantics', () => { }); // @gate enableLegacyCache - // @gate replayFailedUnitOfWorkWithInvokeGuardedCallback it('are properly handled for layout effect creation', async () => { let useLayoutEffectShouldThrow = false; - function ThrowsInLayoutEffect() { + function ThrowsInLayoutEffect({unused}) { Scheduler.log('ThrowsInLayoutEffect render'); React.useLayoutEffect(() => { Scheduler.log('ThrowsInLayoutEffect useLayoutEffect create'); @@ -2117,9 +2116,8 @@ describe('ReactSuspenseEffectsSemantics', () => { }); // @gate enableLegacyCache - // @gate replayFailedUnitOfWorkWithInvokeGuardedCallback it('are properly handled for layout effect destruction', async () => { - function ThrowsInLayoutEffectDestroy() { + function ThrowsInLayoutEffectDestroy({unused}) { Scheduler.log('ThrowsInLayoutEffectDestroy render'); React.useLayoutEffect(() => { Scheduler.log('ThrowsInLayoutEffectDestroy useLayoutEffect create'); @@ -3013,11 +3011,10 @@ describe('ReactSuspenseEffectsSemantics', () => { describe('that throw errors', () => { // @gate enableLegacyCache - // @gate replayFailedUnitOfWorkWithInvokeGuardedCallback it('are properly handled in ref callbacks', async () => { let useRefCallbackShouldThrow = false; - function ThrowsInRefCallback() { + function ThrowsInRefCallback({unused}) { Scheduler.log('ThrowsInRefCallback render'); const refCallback = React.useCallback(value => { Scheduler.log('ThrowsInRefCallback refCallback ref? ' + !!value); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js index ce83b244eeba5..d099b6d72ffb2 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js @@ -3,7 +3,6 @@ let Suspense; let ReactNoop; let Scheduler; let act; -let ReactFeatureFlags; let Random; const SEED = process.env.FUZZ_TEST_SEED || 'default'; @@ -21,9 +20,6 @@ function prettyFormat(thing) { describe('ReactSuspenseFuzz', () => { beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); Suspense = React.Suspense; ReactNoop = require('react-noop-renderer'); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index eed0e69fe1395..a1771263fbd7c 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -28,7 +28,6 @@ describe('ReactSuspensePlaceholder', () => { ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.enableProfilerTimer = true; - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 77aae6ee53d9b..4d1eed5456614 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -398,8 +398,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); // Second condition is redundant but guarantees that the test runs in prod. - // TODO: Delete this feature flag. - // @gate !replayFailedUnitOfWorkWithInvokeGuardedCallback || !__DEV__ // @gate enableLegacyCache it('retries on error', async () => { class ErrorBoundary extends React.Component { @@ -458,8 +456,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); // Second condition is redundant but guarantees that the test runs in prod. - // TODO: Delete this feature flag. - // @gate !replayFailedUnitOfWorkWithInvokeGuardedCallback || !__DEV__ // @gate enableLegacyCache it('retries on error after falling back to a placeholder', async () => { class ErrorBoundary extends React.Component { diff --git a/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js b/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js index 36b9896952b56..139cbfc17e323 100644 --- a/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js +++ b/packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js @@ -11,7 +11,6 @@ 'use strict'; const ReactFeatureFlags = require('shared/ReactFeatureFlags'); -ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; const React = require('react'); const ReactTestRenderer = require('react-test-renderer'); const {format: prettyFormat} = require('pretty-format'); diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee index 6aff89c732d7d..0cc201ea06341 100644 --- a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee @@ -55,9 +55,7 @@ describe 'ReactCoffeeScriptClass', -> root.render React.createElement(Foo) ).toThrow() ).toErrorDev([ - # A failed component renders four times in DEV in concurrent mode - 'No `render` method found on the Foo instance', - 'No `render` method found on the Foo instance', + # A failed component renders twice in DEV in concurrent mode 'No `render` method found on the Foo instance', 'No `render` method found on the Foo instance', ]) diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js index 8b7097b7a1409..326576e525b83 100644 --- a/packages/react/src/__tests__/ReactES6Class-test.js +++ b/packages/react/src/__tests__/ReactES6Class-test.js @@ -63,11 +63,7 @@ describe('ReactES6Class', () => { expect(() => { expect(() => ReactDOM.flushSync(() => root.render())).toThrow(); }).toErrorDev([ - // A failed component renders four times in DEV in concurrent mode - 'Warning: No `render` method found on the Foo instance: ' + - 'you may have forgotten to define `render`.', - 'Warning: No `render` method found on the Foo instance: ' + - 'you may have forgotten to define `render`.', + // A failed component renders twice in DEV in concurrent mode 'Warning: No `render` method found on the Foo instance: ' + 'you may have forgotten to define `render`.', 'Warning: No `render` method found on the Foo instance: ' + diff --git a/packages/react/src/__tests__/ReactElementValidator-test.internal.js b/packages/react/src/__tests__/ReactElementValidator-test.internal.js index a5bb9954a192d..b1d7257ccdb10 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.internal.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.internal.js @@ -19,16 +19,12 @@ let React; let ReactDOMClient; let act; -let ReactFeatureFlags = require('shared/ReactFeatureFlags'); - describe('ReactElementValidator', () => { let ComponentClass; beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 22c551e0801f2..201ef39036970 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -25,7 +25,6 @@ function loadModules({ enableProfilerTimer = true, enableProfilerCommitHooks = true, enableProfilerNestedUpdatePhase = true, - replayFailedUnitOfWorkWithInvokeGuardedCallback = false, } = {}) { ReactFeatureFlags = require('shared/ReactFeatureFlags'); @@ -33,8 +32,6 @@ function loadModules({ ReactFeatureFlags.enableProfilerCommitHooks = enableProfilerCommitHooks; ReactFeatureFlags.enableProfilerNestedUpdatePhase = enableProfilerNestedUpdatePhase; - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = - replayFailedUnitOfWorkWithInvokeGuardedCallback; React = require('react'); Scheduler = require('scheduler'); @@ -1015,205 +1012,170 @@ describe(`onRender`, () => { expect(call[5]).toBe(380); // commit time }); - [true, false].forEach(replayFailedUnitOfWorkWithInvokeGuardedCallback => { - describe(`replayFailedUnitOfWorkWithInvokeGuardedCallback ${ - replayFailedUnitOfWorkWithInvokeGuardedCallback ? 'enabled' : 'disabled' - }`, () => { - beforeEach(() => { - jest.resetModules(); - - loadModules({ - replayFailedUnitOfWorkWithInvokeGuardedCallback, - }); - }); - - it('should accumulate actual time after an error handled by componentDidCatch()', async () => { - const callback = jest.fn(); - - const ThrowsError = ({unused}) => { - Scheduler.unstable_advanceTime(3); - throw Error('expected error'); - }; - - class ErrorBoundary extends React.Component { - state = {error: null}; - componentDidCatch(error) { - this.setState({error}); - } - render() { - Scheduler.unstable_advanceTime(2); - return this.state.error === null ? ( - this.props.children - ) : ( - - ); - } - } + it('should accumulate actual time after an error handled by componentDidCatch()', async () => { + const callback = jest.fn(); - Scheduler.unstable_advanceTime(5); // 0 -> 5 - - await act(() => { - ReactNoop.render( - - - - - - , - ); - }); - - expect(callback).toHaveBeenCalledTimes(2); - - // Callbacks bubble (reverse order). - const [mountCall, updateCall] = callback.mock.calls; - - // The initial mount only includes the ErrorBoundary (which takes 2) - // But it spends time rendering all of the failed subtree also. - expect(mountCall[1]).toBe('mount'); - // actual time includes: 2 (ErrorBoundary) + 9 (AdvanceTime) + 3 (ThrowsError) - // We don't count the time spent in replaying the failed unit of work (ThrowsError) - expect(mountCall[2]).toBe(14); - // base time includes: 2 (ErrorBoundary) - // Since the tree is empty for the initial commit - expect(mountCall[3]).toBe(2); - // start time: 5 initially + 14 of work - // Add an additional 3 (ThrowsError) if we replayed the failed work - expect(mountCall[4]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 22 - : 19, - ); - // commit time: 19 initially + 14 of work - // Add an additional 6 (ThrowsError *2) if we replayed the failed work - expect(mountCall[5]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 39 - : 33, - ); + const ThrowsError = ({unused}) => { + Scheduler.unstable_advanceTime(3); + throw Error('expected error'); + }; - // The update includes the ErrorBoundary and its fallback child - expect(updateCall[1]).toBe('nested-update'); - // actual time includes: 2 (ErrorBoundary) + 20 (AdvanceTime) - expect(updateCall[2]).toBe(22); - // base time includes: 2 (ErrorBoundary) + 20 (AdvanceTime) - expect(updateCall[3]).toBe(22); - // start time - expect(updateCall[4]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 39 - : 33, - ); - // commit time: 33 (startTime) + 2 (ErrorBoundary) + 20 (AdvanceTime) - // Add an additional 6 (ThrowsError *2) if we replayed the failed work - expect(updateCall[5]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 61 - : 55, + class ErrorBoundary extends React.Component { + state = {error: null}; + componentDidCatch(error) { + this.setState({error}); + } + render() { + Scheduler.unstable_advanceTime(2); + return this.state.error === null ? ( + this.props.children + ) : ( + ); - }); - - it('should accumulate actual time after an error handled by getDerivedStateFromError()', async () => { - const callback = jest.fn(); - - const ThrowsError = ({unused}) => { - Scheduler.unstable_advanceTime(10); - throw Error('expected error'); - }; - - class ErrorBoundary extends React.Component { - state = {error: null}; - static getDerivedStateFromError(error) { - return {error}; - } - render() { - Scheduler.unstable_advanceTime(2); - return this.state.error === null ? ( - this.props.children - ) : ( - - ); - } - } + } + } - Scheduler.unstable_advanceTime(5); // 0 -> 5 - - await act(() => { - ReactNoop.render( - - - - - - , - ); - }); - - expect(callback).toHaveBeenCalledTimes(1); - - // Callbacks bubble (reverse order). - const [mountCall] = callback.mock.calls; - - // The initial mount includes the ErrorBoundary's error state, - // But it also spends actual time rendering UI that fails and isn't included. - expect(mountCall[1]).toBe('mount'); - // actual time includes: 2 (ErrorBoundary) + 5 (AdvanceTime) + 10 (ThrowsError) - // Then the re-render: 2 (ErrorBoundary) + 20 (AdvanceTime) - // We don't count the time spent in replaying the failed unit of work (ThrowsError) - expect(mountCall[2]).toBe(39); - // base time includes: 2 (ErrorBoundary) + 20 (AdvanceTime) - expect(mountCall[3]).toBe(22); - // start time - expect(mountCall[4]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 54 - : 44, - ); - // commit time - expect(mountCall[5]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 103 - : 83, - ); - }); + Scheduler.unstable_advanceTime(5); // 0 -> 5 + + const root = ReactNoop.createRoot(); + await act(() => { + root.render( + + + + + + , + ); + }); - it('should reset the fiber stack correct after a "complete" phase error', async () => { - jest.resetModules(); + expect(callback).toHaveBeenCalledTimes(2); - loadModules({ - replayFailedUnitOfWorkWithInvokeGuardedCallback, - }); + // Callbacks bubble (reverse order). + const [mountCall, updateCall] = callback.mock.calls; + + // The initial mount only includes the ErrorBoundary (which takes 2) + // But it spends time rendering all of the failed subtree also. + expect(mountCall[1]).toBe('mount'); + // actual time includes: 2 (ErrorBoundary) + 9 (AdvanceTime) + 3 (ThrowsError) + // We don't count the time spent in replaying the failed unit of work (ThrowsError) + expect(mountCall[2]).toBe(14); + // base time includes: 2 (ErrorBoundary) + // Since the tree is empty for the initial commit + expect(mountCall[3]).toBe(2); + + // start time: 5 initially + 14 of work + // Add an additional 3 (ThrowsError) if we replayed the failed work + expect(mountCall[4]).toBe(19); + // commit time: 19 initially + 14 of work + // Add an additional 6 (ThrowsError *2) if we replayed the failed work + expect(mountCall[5]).toBe(33); + + // The update includes the ErrorBoundary and its fallback child + expect(updateCall[1]).toBe('nested-update'); + // actual time includes: 2 (ErrorBoundary) + 20 (AdvanceTime) + expect(updateCall[2]).toBe(22); + // base time includes: 2 (ErrorBoundary) + 20 (AdvanceTime) + expect(updateCall[3]).toBe(22); + // start time + expect(updateCall[4]).toBe(33); + // commit time: 19 (startTime) + 2 (ErrorBoundary) + 20 (AdvanceTime) + // Add an additional 3 (ThrowsError) if we replayed the failed work + expect(updateCall[5]).toBe(55); + }); - // Simulate a renderer error during the "complete" phase. - // This mimics behavior like React Native's View/Text nesting validation. - ReactNoop.render( - - hi - , - ); - await waitForThrow('Error in host config.'); - - // A similar case we've seen caused by an invariant in ReactDOM. - // It didn't reproduce without a host component inside. - ReactNoop.render( - - - hi - - , - ); - await waitForThrow('Error in host config.'); - - // So long as the profiler timer's fiber stack is reset correctly, - // Subsequent renders should not error. - ReactNoop.render( - - hi - , + it('should accumulate actual time after an error handled by getDerivedStateFromError()', async () => { + const callback = jest.fn(); + + const ThrowsError = ({unused}) => { + Scheduler.unstable_advanceTime(10); + throw Error('expected error'); + }; + + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + return {error}; + } + render() { + Scheduler.unstable_advanceTime(2); + return this.state.error === null ? ( + this.props.children + ) : ( + ); - await waitForAll([]); - }); + } + } + + Scheduler.unstable_advanceTime(5); // 0 -> 5 + + await act(() => { + const root = ReactNoop.createRoot(); + root.render( + + + + + + , + ); }); + + expect(callback).toHaveBeenCalledTimes(1); + + // Callbacks bubble (reverse order). + const [mountCall] = callback.mock.calls; + + // The initial mount includes the ErrorBoundary's error state, + // But it also spends actual time rendering UI that fails and isn't included. + expect(mountCall[1]).toBe('mount'); + // actual time includes: 2 (ErrorBoundary) + 5 (AdvanceTime) + 10 (ThrowsError) + // Then the re-render: 2 (ErrorBoundary) + 20 (AdvanceTime) + // We don't count the time spent in replaying the failed unit of work (ThrowsError) + expect(mountCall[2]).toBe(39); + // base time includes: 2 (ErrorBoundary) + 20 (AdvanceTime) + expect(mountCall[3]).toBe(22); + // start time + expect(mountCall[4]).toBe(44); + // commit time + expect(mountCall[5]).toBe(83); + }); + + it('should reset the fiber stack correct after a "complete" phase error', async () => { + jest.resetModules(); + + loadModules({ + useNoopRenderer: true, + }); + + // Simulate a renderer error during the "complete" phase. + // This mimics behavior like React Native's View/Text nesting validation. + ReactNoop.render( + + hi + , + ); + await waitForThrow('Error in host config.'); + + // A similar case we've seen caused by an invariant in ReactDOM. + // It didn't reproduce without a host component inside. + ReactNoop.render( + + + hi + + , + ); + await waitForThrow('Error in host config.'); + + // So long as the profiler timer's fiber stack is reset correctly, + // Subsequent renders should not error. + ReactNoop.render( + + hi + , + ); + await waitForAll([]); }); }); diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts index b79886fe72b9c..fd2e8c7ee0ecc 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -332,11 +332,7 @@ describe('ReactTypeScriptClass', function() { ReactDOM.flushSync(() => root.render(React.createElement(Empty))) ).toThrow(); }).toErrorDev([ - // A failed component renders four times in DEV in concurrent mode - 'Warning: No `render` method found on the Empty instance: ' + - 'you may have forgotten to define `render`.', - 'Warning: No `render` method found on the Empty instance: ' + - 'you may have forgotten to define `render`.', + // A failed component renders twice in DEV in concurrent mode 'Warning: No `render` method found on the Empty instance: ' + 'you may have forgotten to define `render`.', 'Warning: No `render` method found on the Empty instance: ' + diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index 6da58ee051fc4..42bbfe44d32ce 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -11,16 +11,13 @@ describe('forwardRef', () => { let React; - let ReactFeatureFlags; let ReactNoop; let Scheduler; let waitForAll; beforeEach(() => { jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); diff --git a/packages/shared/ReactErrorUtils.js b/packages/shared/ReactErrorUtils.js deleted file mode 100644 index b2d9cde7b7124..0000000000000 --- a/packages/shared/ReactErrorUtils.js +++ /dev/null @@ -1,125 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import invokeGuardedCallbackImpl from './invokeGuardedCallbackImpl'; - -// Used by Fiber to simulate a try-catch. -let hasError: boolean = false; -let caughtError: mixed = null; - -// Used by event system to capture/rethrow the first error. -let hasRethrowError: boolean = false; -let rethrowError: mixed = null; - -const reporter = { - onError(error: mixed) { - hasError = true; - caughtError = error; - }, -}; - -/** - * Call a function while guarding against errors that happens within it. - * Returns an error if it throws, otherwise null. - * - * In production, this is implemented using a try-catch. The reason we don't - * use a try-catch directly is so that we can swap out a different - * implementation in DEV mode. - * - * @param {String} name of the guard to use for logging or debugging - * @param {Function} func The function to invoke - * @param {*} context The context to use when calling the function - * @param {...*} args Arguments for function - */ -export function invokeGuardedCallback( - name: string | null, - func: (a: A, b: B, c: C, d: D, e: E, f: F) => mixed, - context: Context, - a: A, - b: B, - c: C, - d: D, - e: E, - f: F, -): void { - hasError = false; - caughtError = null; - invokeGuardedCallbackImpl.apply(reporter, arguments); -} - -/** - * Same as invokeGuardedCallback, but instead of returning an error, it stores - * it in a global so it can be rethrown by `rethrowCaughtError` later. - * TODO: See if caughtError and rethrowError can be unified. - * - * @param {String} name of the guard to use for logging or debugging - * @param {Function} func The function to invoke - * @param {*} context The context to use when calling the function - * @param {...*} args Arguments for function - */ -export function invokeGuardedCallbackAndCatchFirstError< - A, - B, - C, - D, - E, - F, - Context, ->( - this: mixed, - name: string | null, - func: (a: A, b: B, c: C, d: D, e: E, f: F) => void, - context: Context, - a: A, - b: B, - c: C, - d: D, - e: E, - f: F, -): void { - invokeGuardedCallback.apply(this, arguments); - if (hasError) { - const error = clearCaughtError(); - if (!hasRethrowError) { - hasRethrowError = true; - rethrowError = error; - } - } -} - -/** - * During execution of guarded functions we will capture the first error which - * we will rethrow to be handled by the top level error handler. - */ -export function rethrowCaughtError() { - if (hasRethrowError) { - const error = rethrowError; - hasRethrowError = false; - rethrowError = null; - throw error; - } -} - -export function hasCaughtError(): boolean { - return hasError; -} - -export function clearCaughtError(): mixed { - if (hasError) { - const error = caughtError; - hasError = false; - caughtError = null; - return error; - } else { - throw new Error( - 'clearCaughtError was called but no error was captured. This error ' + - 'is likely caused by a bug in React. Please file an issue.', - ); - } -} diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 4f8fde4a262a6..cdd57a308380c 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -251,10 +251,6 @@ export const enableSchedulingProfiler = __PROFILE__; // reducers by double invoking them in StrictLegacyMode. export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; -// To preserve the "Pause on caught exceptions" behavior of the debugger, we -// replay the begin phase of a failed component inside invokeGuardedCallback. -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; - // Gather advanced timing metrics for Profiler subtrees. export const enableProfilerTimer = __PROFILE__; diff --git a/packages/shared/__tests__/ReactErrorUtils-test.internal.js b/packages/shared/__tests__/ReactErrorUtils-test.internal.js deleted file mode 100644 index f0bf1197c8ed8..0000000000000 --- a/packages/shared/__tests__/ReactErrorUtils-test.internal.js +++ /dev/null @@ -1,210 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @emails react-core - */ - -'use strict'; - -let ReactErrorUtils; - -describe('ReactErrorUtils', () => { - beforeEach(() => { - // TODO: can we express this test with only public API? - ReactErrorUtils = require('shared/ReactErrorUtils'); - }); - - it(`it should rethrow caught errors`, () => { - const err = new Error('foo'); - const callback = function () { - throw err; - }; - ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError( - 'foo', - callback, - null, - ); - expect(ReactErrorUtils.hasCaughtError()).toBe(false); - expect(() => ReactErrorUtils.rethrowCaughtError()).toThrow(err); - }); - - it(`should call the callback the passed arguments`, () => { - const callback = jest.fn(); - ReactErrorUtils.invokeGuardedCallback( - 'foo', - callback, - null, - 'arg1', - 'arg2', - ); - expect(callback).toBeCalledWith('arg1', 'arg2'); - }); - - it(`should call the callback with the provided context`, () => { - const context = {didCall: false}; - ReactErrorUtils.invokeGuardedCallback( - 'foo', - function () { - this.didCall = true; - }, - context, - ); - expect(context.didCall).toBe(true); - }); - - it(`should catch errors`, () => { - const error = new Error(); - const returnValue = ReactErrorUtils.invokeGuardedCallback( - 'foo', - function () { - throw error; - }, - null, - 'arg1', - 'arg2', - ); - expect(returnValue).toBe(undefined); - expect(ReactErrorUtils.hasCaughtError()).toBe(true); - expect(ReactErrorUtils.clearCaughtError()).toBe(error); - }); - - it(`should return false from clearCaughtError if no error was thrown`, () => { - const callback = jest.fn(); - ReactErrorUtils.invokeGuardedCallback('foo', callback, null); - expect(ReactErrorUtils.hasCaughtError()).toBe(false); - expect(ReactErrorUtils.clearCaughtError).toThrow('no error was captured'); - }); - - it(`can nest with same debug name`, () => { - const err1 = new Error(); - let err2; - const err3 = new Error(); - ReactErrorUtils.invokeGuardedCallback( - 'foo', - function () { - ReactErrorUtils.invokeGuardedCallback( - 'foo', - function () { - throw err1; - }, - null, - ); - err2 = ReactErrorUtils.clearCaughtError(); - throw err3; - }, - null, - ); - const err4 = ReactErrorUtils.clearCaughtError(); - - expect(err2).toBe(err1); - expect(err4).toBe(err3); - }); - - it(`handles nested errors`, () => { - const err1 = new Error(); - let err2; - ReactErrorUtils.invokeGuardedCallback( - 'foo', - function () { - ReactErrorUtils.invokeGuardedCallback( - 'foo', - function () { - throw err1; - }, - null, - ); - err2 = ReactErrorUtils.clearCaughtError(); - }, - null, - ); - // Returns null because inner error was already captured - expect(ReactErrorUtils.hasCaughtError()).toBe(false); - - expect(err2).toBe(err1); - }); - - it('handles nested errors in separate renderers', () => { - const ReactErrorUtils1 = require('shared/ReactErrorUtils'); - jest.resetModules(); - const ReactErrorUtils2 = require('shared/ReactErrorUtils'); - expect(ReactErrorUtils1).not.toEqual(ReactErrorUtils2); - - const ops = []; - - ReactErrorUtils1.invokeGuardedCallback( - null, - () => { - ReactErrorUtils2.invokeGuardedCallback( - null, - () => { - throw new Error('nested error'); - }, - null, - ); - // ReactErrorUtils2 should catch the error - ops.push(ReactErrorUtils2.hasCaughtError()); - ops.push(ReactErrorUtils2.clearCaughtError().message); - }, - null, - ); - - // ReactErrorUtils1 should not catch the error - ops.push(ReactErrorUtils1.hasCaughtError()); - - expect(ops).toEqual([true, 'nested error', false]); - }); - - if (!__DEV__) { - // jsdom doesn't handle this properly, but Chrome and Firefox should. Test - // this with a fixture. - it('catches null values', () => { - ReactErrorUtils.invokeGuardedCallback( - null, - function () { - throw null; // eslint-disable-line no-throw-literal - }, - null, - ); - expect(ReactErrorUtils.hasCaughtError()).toBe(true); - expect(ReactErrorUtils.clearCaughtError()).toBe(null); - }); - } - - it(`can be shimmed`, () => { - const ops = []; - jest.resetModules(); - jest.mock( - 'shared/invokeGuardedCallbackImpl', - () => - function invokeGuardedCallback(name, func, context, a) { - ops.push(a); - try { - func.call(context, a); - } catch (error) { - this.onError(error); - } - }, - ); - ReactErrorUtils = require('shared/ReactErrorUtils'); - - try { - const err = new Error('foo'); - const callback = function () { - throw err; - }; - ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError( - 'foo', - callback, - null, - 'somearg', - ); - expect(() => ReactErrorUtils.rethrowCaughtError()).toThrow(err); - expect(ops).toEqual(['somearg']); - } finally { - jest.unmock('shared/invokeGuardedCallbackImpl'); - } - }); -}); diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 256bb743bdf58..a9e0348ebad76 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -54,7 +54,6 @@ export const disableJavaScriptURLs = true; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const disableIEWorkarounds = true; -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 166745684951c..6f487a7c275e7 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -14,7 +14,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const enableDebugTracing = false; export const enableAsyncDebugInfo = false; export const enableSchedulingProfiler = false; -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 4d9dd958f1ca6..981f7bf10572f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -14,7 +14,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false; export const enableDebugTracing = false; export const enableAsyncDebugInfo = false; export const enableSchedulingProfiler = false; -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index cbb6d4034cf3c..ad8a03ae72438 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -14,7 +14,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false; export const enableDebugTracing = false; export const enableAsyncDebugInfo = false; export const enableSchedulingProfiler = false; -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 0ec325a4997af..8a33eb72270a6 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -14,7 +14,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false; export const enableDebugTracing = false; export const enableAsyncDebugInfo = false; export const enableSchedulingProfiler = false; -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false; export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index d3f28ec94c57e..7fa47bfe0e83d 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -44,10 +44,6 @@ export const enableSchedulingProfiler = __VARIANT__; export const enableInfiniteRenderLoopDetection = __VARIANT__; -// These are already tested in both modes using the build type dimension, -// so we don't need to use __VARIANT__ to get extra coverage. -export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; - // TODO: These flags are hard-coded to the default values used in open source. // Update the tests so that they pass in either mode, then set these // to __VARIANT__. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index f5d9bc00c99fb..ffe591425a336 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -18,7 +18,6 @@ export const { disableInputAttributeSyncing, disableIEWorkarounds, enableTrustedTypesIntegration, - replayFailedUnitOfWorkWithInvokeGuardedCallback, enableLegacyFBSupport, enableDebugTracing, enableUseRefAccessWarning, diff --git a/packages/shared/forks/invokeGuardedCallbackImpl.www.js b/packages/shared/forks/invokeGuardedCallbackImpl.www.js deleted file mode 100644 index b85c10e7a4c07..0000000000000 --- a/packages/shared/forks/invokeGuardedCallbackImpl.www.js +++ /dev/null @@ -1,34 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @noflow - */ - -// Provided by www -const ReactFbErrorUtils = require('ReactFbErrorUtils'); - -if (typeof ReactFbErrorUtils.invokeGuardedCallback !== 'function') { - throw new Error( - 'Expected ReactFbErrorUtils.invokeGuardedCallback to be a function.', - ); -} - -function invokeGuardedCallbackImpl( - name: string | null, - func: (a: A, b: B, c: C, d: D, e: E, f: F) => mixed, - context: Context, - a: A, - b: B, - c: C, - d: D, - e: E, - f: F, -) { - // This will call `this.onError(err)` if an error was caught. - ReactFbErrorUtils.invokeGuardedCallback.apply(this, arguments); -} - -export default invokeGuardedCallbackImpl; diff --git a/packages/shared/invokeGuardedCallbackImpl.js b/packages/shared/invokeGuardedCallbackImpl.js deleted file mode 100644 index 745f32c2a8829..0000000000000 --- a/packages/shared/invokeGuardedCallbackImpl.js +++ /dev/null @@ -1,215 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -let fakeNode: Element = (null: any); -if (__DEV__) { - if ( - typeof window !== 'undefined' && - typeof window.dispatchEvent === 'function' && - typeof document !== 'undefined' && - // $FlowFixMe[method-unbinding] - typeof document.createEvent === 'function' - ) { - fakeNode = document.createElement('react'); - } -} - -export default function invokeGuardedCallbackImpl, Context>( - this: {onError: (error: mixed) => void}, - name: string | null, - func: (...Args) => mixed, - context: Context, -): void { - if (__DEV__) { - // In DEV mode, we use a special version - // that plays more nicely with the browser's DevTools. The idea is to preserve - // "Pause on exceptions" behavior. Because React wraps all user-provided - // functions in invokeGuardedCallback, and the production version of - // invokeGuardedCallback uses a try-catch, all user exceptions are treated - // like caught exceptions, and the DevTools won't pause unless the developer - // takes the extra step of enabling pause on caught exceptions. This is - // unintuitive, though, because even though React has caught the error, from - // the developer's perspective, the error is uncaught. - // - // To preserve the expected "Pause on exceptions" behavior, we don't use a - // try-catch in DEV. Instead, we synchronously dispatch a fake event to a fake - // DOM node, and call the user-provided callback from inside an event handler - // for that fake event. If the callback throws, the error is "captured" using - // event loop context, it does not interrupt the normal program flow. - // Effectively, this gives us try-catch behavior without actually using - // try-catch. Neat! - - // fakeNode signifies we are in an environment with a document and window object - if (fakeNode) { - const evt = document.createEvent('Event'); - - let didCall = false; - // Keeps track of whether the user-provided callback threw an error. We - // set this to true at the beginning, then set it to false right after - // calling the function. If the function errors, `didError` will never be - // set to false. This strategy works even if the browser is flaky and - // fails to call our global error handler, because it doesn't rely on - // the error event at all. - let didError = true; - - // Keeps track of the value of window.event so that we can reset it - // during the callback to let user code access window.event in the - // browsers that support it. - const windowEvent = window.event; - - // Keeps track of the descriptor of window.event to restore it after event - // dispatching: https://github.com/facebook/react/issues/13688 - const windowEventDescriptor = Object.getOwnPropertyDescriptor( - window, - 'event', - ); - - const restoreAfterDispatch = () => { - // We immediately remove the callback from event listeners so that - // nested `invokeGuardedCallback` calls do not clash. Otherwise, a - // nested call would trigger the fake event handlers of any call higher - // in the stack. - fakeNode.removeEventListener(evtType, callCallback, false); - - // We check for window.hasOwnProperty('event') to prevent the - // window.event assignment in both IE <= 10 as they throw an error - // "Member not found" in strict mode, and in Firefox which does not - // support window.event. - if ( - typeof window.event !== 'undefined' && - window.hasOwnProperty('event') - ) { - window.event = windowEvent; - } - }; - - // Create an event handler for our fake event. We will synchronously - // dispatch our fake event using `dispatchEvent`. Inside the handler, we - // call the user-provided callback. - // $FlowFixMe[method-unbinding] - const funcArgs = Array.prototype.slice.call(arguments, 3); - const callCallback = () => { - didCall = true; - restoreAfterDispatch(); - // $FlowFixMe[incompatible-call] Flow doesn't understand the arguments splicing. - func.apply(context, funcArgs); - didError = false; - }; - - // Create a global error event handler. We use this to capture the value - // that was thrown. It's possible that this error handler will fire more - // than once; for example, if non-React code also calls `dispatchEvent` - // and a handler for that event throws. We should be resilient to most of - // those cases. Even if our error event handler fires more than once, the - // last error event is always used. If the callback actually does error, - // we know that the last error event is the correct one, because it's not - // possible for anything else to have happened in between our callback - // erroring and the code that follows the `dispatchEvent` call below. If - // the callback doesn't error, but the error event was fired, we know to - // ignore it because `didError` will be false, as described above. - let error; - // Use this to track whether the error event is ever called. - let didSetError = false; - let isCrossOriginError = false; - - const handleWindowError = (event: ErrorEvent) => { - error = event.error; - didSetError = true; - if (error === null && event.colno === 0 && event.lineno === 0) { - isCrossOriginError = true; - } - if (event.defaultPrevented) { - // Some other error handler has prevented default. - // Browsers silence the error report if this happens. - // We'll remember this to later decide whether to log it or not. - if (error != null && typeof error === 'object') { - try { - error._suppressLogging = true; - } catch (inner) { - // Ignore. - } - } - } - }; - - // Create a fake event type. - const evtType = `react-${name ? name : 'invokeguardedcallback'}`; - - // Attach our event handlers - window.addEventListener('error', handleWindowError); - fakeNode.addEventListener(evtType, callCallback, false); - - // Synchronously dispatch our fake event. If the user-provided function - // errors, it will trigger our global error handler. - evt.initEvent(evtType, false, false); - fakeNode.dispatchEvent(evt); - if (windowEventDescriptor) { - Object.defineProperty(window, 'event', windowEventDescriptor); - } - - if (didCall && didError) { - if (!didSetError) { - // The callback errored, but the error event never fired. - // eslint-disable-next-line react-internal/prod-error-codes - error = new Error( - 'An error was thrown inside one of your components, but React ' + - "doesn't know what it was. This is likely due to browser " + - 'flakiness. React does its best to preserve the "Pause on ' + - 'exceptions" behavior of the DevTools, which requires some ' + - "DEV-mode only tricks. It's possible that these don't work in " + - 'your browser. Try triggering the error in production mode, ' + - 'or switching to a modern browser. If you suspect that this is ' + - 'actually an issue with React, please file an issue.', - ); - } else if (isCrossOriginError) { - // eslint-disable-next-line react-internal/prod-error-codes - error = new Error( - "A cross-origin error was thrown. React doesn't have access to " + - 'the actual error object in development. ' + - 'See https://react.dev/link/crossorigin-error for more information.', - ); - } - this.onError(error); - } - - // Remove our event listeners - window.removeEventListener('error', handleWindowError); - - if (didCall) { - return; - } else { - // Something went really wrong, and our event was not dispatched. - // https://github.com/facebook/react/issues/16734 - // https://github.com/facebook/react/issues/16585 - // Fall back to the production implementation. - restoreAfterDispatch(); - // we fall through and call the prod version instead - } - } - // We only get here if we are in an environment that either does not support the browser - // variant or we had trouble getting the browser to emit the error. - // $FlowFixMe[method-unbinding] - const funcArgs = Array.prototype.slice.call(arguments, 3); - try { - // $FlowFixMe[incompatible-call] Flow doesn't understand the arguments splicing. - func.apply(context, funcArgs); - } catch (error) { - this.onError(error); - } - } else { - // $FlowFixMe[method-unbinding] - const funcArgs = Array.prototype.slice.call(arguments, 3); - try { - // $FlowFixMe[incompatible-call] Flow doesn't understand the arguments splicing. - func.apply(context, funcArgs); - } catch (error) { - this.onError(error); - } - } -} diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index 2fe71bf73c528..5d460444440d8 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -22,6 +22,8 @@ let useEffect; let useLayoutEffect; let assertLog; +let originalError; + // This tests shared behavior between the built-in and shim implementations of // of useSyncExternalStore. describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { @@ -44,6 +46,9 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { : 'react-dom-17/umd/react-dom.production.min.js', ), ); + // Because React 17 prints extra logs we need to ignore them. + originalError = console.error; + console.error = jest.fn(); } React = require('react'); @@ -82,6 +87,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { require('use-sync-external-store/shim/with-selector').useSyncExternalStoreWithSelector; }); + afterEach(() => { + if (gate(flags => flags.enableUseSyncExternalStoreShim)) { + console.error = originalError; + } + }); + function Text({text}) { Scheduler.log(text); return text; @@ -593,12 +604,20 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 'the number of nested updates to prevent infinite loops.', ); }).toErrorDev( - 'The result of getSnapshot should be cached to avoid an infinite loop', + gate(flags => flags.enableUseSyncExternalStoreShim) + ? [ + 'Uncaught [', + 'The result of getSnapshot should be cached to avoid an infinite loop', + 'The above error occurred in the', + ] + : [ + 'The result of getSnapshot should be cached to avoid an infinite loop', + ], { withoutStack: gate(flags => { if (flags.enableUseSyncExternalStoreShim) { // Stacks don't work when mixing the source and the npm package. - return flags.source; + return flags.source ? 1 : 0; } return false; }), diff --git a/scripts/jest/shouldIgnoreConsoleError.js b/scripts/jest/shouldIgnoreConsoleError.js index 79ec7fc2ad7d6..2627e927eb892 100644 --- a/scripts/jest/shouldIgnoreConsoleError.js +++ b/scripts/jest/shouldIgnoreConsoleError.js @@ -7,14 +7,13 @@ module.exports = function shouldIgnoreConsoleError( ) { if (__DEV__) { if (typeof format === 'string') { - if (format.indexOf('Error: Uncaught [') === 0) { - // This looks like an uncaught error from invokeGuardedCallback() wrapper - // in development that is reported by jsdom. Ignore because it's noisy. - return true; - } - if (format.indexOf('The above error occurred') === 0) { - // This looks like an error addendum from ReactFiberErrorLogger. - // Ignore it too. + if ( + args[0] != null && + typeof args[0].message === 'string' && + typeof args[0].stack === 'string' + ) { + // This looks like an error with addendum from ReactFiberErrorLogger. + // They are noisy too so we'll try to ignore them. return true; } if ( @@ -36,17 +35,6 @@ module.exports = function shouldIgnoreConsoleError( // This also gets logged by onRecoverableError, so we can ignore it. return true; } - } else if ( - format != null && - typeof format.message === 'string' && - typeof format.stack === 'string' && - args.length === 0 - ) { - if (format.stack.indexOf('Error: Uncaught [') === 0) { - // This looks like an uncaught error from invokeGuardedCallback() wrapper - // in development that is reported by jest-environment-jsdom. Ignore because it's noisy. - return true; - } } } else { if ( diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js index 10a6324b23bb1..eef64866e1932 100644 --- a/scripts/rollup/forks.js +++ b/scripts/rollup/forks.js @@ -232,18 +232,6 @@ const forks = Object.freeze({ } }, - // Different wrapping/reporting for caught errors. - './packages/shared/invokeGuardedCallbackImpl.js': (bundleType, entry) => { - switch (bundleType) { - case FB_WWW_DEV: - case FB_WWW_PROD: - case FB_WWW_PROFILING: - return './packages/shared/forks/invokeGuardedCallbackImpl.www.js'; - default: - return null; - } - }, - // Different dialogs for caught errors. './packages/react-reconciler/src/ReactFiberErrorDialog.js': ( bundleType,