From de66616e93195781b74b87c07fce8243577f1836 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 7 Mar 2024 00:43:31 -0500 Subject: [PATCH] Remove invokeGuardedCallback and replay trick --- .../__tests__/ReactCacheOld-test.internal.js | 4 - .../__tests__/ReactCompositeComponent-test.js | 15 - .../ReactDOMConsoleErrorReporting-test.js | 219 ++----------- .../src/__tests__/ReactDOMFizzServer-test.js | 28 +- ...tDOMServerIntegrationLegacyContext-test.js | 2 +- ...tionLegacyContextDisabled-test.internal.js | 2 +- .../ReactDOMServerIntegrationSelect-test.js | 4 +- ...DOMServerPartialHydration-test.internal.js | 5 +- .../ReactErrorBoundaries-test.internal.js | 3 - ...eactLegacyErrorBoundaries-test.internal.js | 3 - packages/react-reconciler/src/ReactFiber.js | 54 --- .../src/ReactFiberCommitWork.js | 10 +- .../src/ReactFiberHydrationContext.js | 7 - .../src/ReactFiberWorkLoop.js | 97 +----- ...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 | 3 - .../ReactIncrementalErrorLogging-test.js | 2 - ...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 | 310 ++++++++---------- .../__tests__/ReactTypeScriptClass-test.ts | 6 +- .../src/__tests__/forwardRef-test.internal.js | 3 - packages/shared/ReactErrorUtils.js | 27 +- 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 ------------ scripts/jest/shouldIgnoreConsoleError.js | 2 + scripts/rollup/forks.js | 12 - 48 files changed, 209 insertions(+), 1251 deletions(-) delete mode 100644 packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.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/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-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..41a9cffaa1a35 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -82,64 +82,25 @@ 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(); @@ -168,41 +129,8 @@ 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: - 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', - }), - ], [ // Addendum by React: expect.stringContaining( @@ -254,41 +182,8 @@ 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. - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], [ // Addendum by React: expect.stringContaining( @@ -340,24 +235,8 @@ 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: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], [ // Addendum by React: expect.stringContaining( @@ -412,24 +291,8 @@ 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: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], [ // Addendum by React: expect.stringContaining( @@ -438,7 +301,7 @@ describe('ReactDOMConsoleErrorReporting', () => { ], ]); } 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,24 +344,8 @@ 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: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], [ // Addendum by React: expect.stringContaining( @@ -507,7 +354,7 @@ describe('ReactDOMConsoleErrorReporting', () => { ], ]); } 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,24 +400,8 @@ 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: - expect.objectContaining({ - detail: expect.objectContaining({ - message: 'Boom', - }), - type: 'unhandled exception', - }), - ], [ // Addendum by React: expect.stringContaining( diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 0d49e7bae122a..f2734d3175a49 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,7 +4643,6 @@ describe('ReactDOMFizzServer', () => { await waitForAll([]); }); - // @gate __DEV__ it('does not invokeGuardedCallback for errors after the first hydration error', async () => { // We can't use the toErrorDev helper here because this is async. const originalConsoleError = console.error; @@ -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,7 +4740,6 @@ describe('ReactDOMFizzServer', () => { } }); - // @gate __DEV__ it('does not invokeGuardedCallback for errors after a preceding fiber suspends', async () => { // We can't use the toErrorDev helper here because this is async. const originalConsoleError = console.error; @@ -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__/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 5b10d81f2220d..a74272b41020b 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..4170168ddc822 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'); diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index d16d6253e36d1..960ba074c1ea1 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'); 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..8ee7165e3f645 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, @@ -251,10 +250,11 @@ export function reportUncaughtErrorInDEV(error: mixed) { // (https://github.com/facebook/react/issues/21712). // As a compromise, rethrow only caught errors in a guard. if (__DEV__) { - invokeGuardedCallback(null, () => { - throw error; - }); - clearCaughtError(); + // TODO: This trick no longer works. Should probably use reportError maybe. + // invokeGuardedCallback(null, () => { + // throw error; + // }); + // clearCaughtError(); } } diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index 61be1aa39b940..abb5c2b790e11 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 98e164e0b221f..267ea479cd099 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, @@ -78,16 +77,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, @@ -175,7 +167,7 @@ import { import {requestCurrentTransition} from './ReactFiberTransition'; import { SelectiveHydrationException, - beginWork as originalBeginWork, + beginWork, replayFunctionComponent, } from './ReactFiberBeginWork'; import {completeWork} from './ReactFiberCompleteWork'; @@ -239,11 +231,6 @@ import { resetCurrentFiber as resetCurrentDebugFiberInDEV, setCurrentFiber as setCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; -import { - invokeGuardedCallback, - hasCaughtError, - clearCaughtError, -} from 'shared/ReactErrorUtils'; import { isDevToolsPresent, markCommitStarted, @@ -3920,84 +3907,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..f519b0710ffa0 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'); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index e56d0f30d7bce..adfeb238695dd 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -194,11 +194,9 @@ 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), 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 fd42561393101..7ad511f59181c 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -27,7 +27,6 @@ function loadModules({ enableProfilerCommitHooks = true, enableProfilerNestedUpdatePhase = true, enableProfilerNestedUpdateScheduledHook = false, - replayFailedUnitOfWorkWithInvokeGuardedCallback = false, useNoopRenderer = false, } = {}) { ReactFeatureFlags = require('shared/ReactFeatureFlags'); @@ -38,8 +37,6 @@ function loadModules({ enableProfilerNestedUpdatePhase; ReactFeatureFlags.enableProfilerNestedUpdateScheduledHook = enableProfilerNestedUpdateScheduledHook; - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = - replayFailedUnitOfWorkWithInvokeGuardedCallback; React = require('react'); Scheduler = require('scheduler'); @@ -1073,193 +1070,162 @@ describe(`onRender`, () => { expect(call[5]).toBe(380); // commit time }); - [true, false].forEach(replayFailedUnitOfWorkWithInvokeGuardedCallback => { - describe(`replayFailedUnitOfWorkWithInvokeGuardedCallback ${ - replayFailedUnitOfWorkWithInvokeGuardedCallback ? 'enabled' : 'disabled' - }`, () => { - beforeEach(() => { - jest.resetModules(); + it('should accumulate actual time after an error handled by componentDidCatch()', () => { + const callback = jest.fn(); - loadModules({ - replayFailedUnitOfWorkWithInvokeGuardedCallback, - }); - }); + const ThrowsError = ({unused}) => { + Scheduler.unstable_advanceTime(3); + throw Error('expected error'); + }; - it('should accumulate actual time after an error handled by componentDidCatch()', () => { - const callback = jest.fn(); + 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 + ) : ( + + ); + } + } - const ThrowsError = ({unused}) => { - Scheduler.unstable_advanceTime(3); - throw Error('expected error'); - }; + Scheduler.unstable_advanceTime(5); // 0 -> 5 - 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 - ) : ( - - ); - } - } + ReactTestRenderer.create( + + + + + + , + ); - Scheduler.unstable_advanceTime(5); // 0 -> 5 + expect(callback).toHaveBeenCalledTimes(2); - ReactTestRenderer.create( - - - - - - , - ); + // 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 + expect(mountCall[4]).toBe(5); + // commit time: 5 initially + 14 of work + // Add an additional 3 (ThrowsError) if we replayed the failed work + expect(mountCall[5]).toBe(19); + + // 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(19); + // commit time: 19 (startTime) + 2 (ErrorBoundary) + 20 (AdvanceTime) + // Add an additional 3 (ThrowsError) if we replayed the failed work + expect(updateCall[5]).toBe(41); + }); - 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 - expect(mountCall[4]).toBe(5); - // commit time: 5 initially + 14 of work - // Add an additional 3 (ThrowsError) if we replayed the failed work - expect(mountCall[5]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 22 - : 19, - ); + it('should accumulate actual time after an error handled by getDerivedStateFromError()', () => { + const callback = jest.fn(); - // 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 - ? 22 - : 19, - ); - // commit time: 19 (startTime) + 2 (ErrorBoundary) + 20 (AdvanceTime) - // Add an additional 3 (ThrowsError) if we replayed the failed work - expect(updateCall[5]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 44 - : 41, - ); - }); + const ThrowsError = ({unused}) => { + Scheduler.unstable_advanceTime(10); + throw Error('expected error'); + }; - it('should accumulate actual time after an error handled by getDerivedStateFromError()', () => { - const callback = jest.fn(); + 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 + ) : ( + + ); + } + } - const ThrowsError = ({unused}) => { - Scheduler.unstable_advanceTime(10); - throw Error('expected error'); - }; + Scheduler.unstable_advanceTime(5); // 0 -> 5 - 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 - ) : ( - - ); - } - } + ReactTestRenderer.create( + + + + + + , + ); - Scheduler.unstable_advanceTime(5); // 0 -> 5 + expect(callback).toHaveBeenCalledTimes(1); - ReactTestRenderer.create( - - - - - - , - ); + // 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(5); + // commit time + expect(mountCall[5]).toBe(44); + }); - 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(5); - // commit time - expect(mountCall[5]).toBe( - __DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback - ? 54 - : 44, - ); - }); + it('should reset the fiber stack correct after a "complete" phase error', async () => { + jest.resetModules(); - it('should reset the fiber stack correct after a "complete" phase error', async () => { - jest.resetModules(); + loadModules({ + useNoopRenderer: true, + }); - loadModules({ - useNoopRenderer: true, - replayFailedUnitOfWorkWithInvokeGuardedCallback, - }); + // 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.'); - // 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.'); + // 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([]); - }); - }); + // 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 9db1627c4346c..7d2195c9d68ff 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -333,11 +333,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 index b2d9cde7b7124..b5a7982c84bd9 100644 --- a/packages/shared/ReactErrorUtils.js +++ b/packages/shared/ReactErrorUtils.js @@ -7,8 +7,6 @@ * @flow */ -import invokeGuardedCallbackImpl from './invokeGuardedCallbackImpl'; - // Used by Fiber to simulate a try-catch. let hasError: boolean = false; let caughtError: mixed = null; @@ -17,13 +15,6 @@ let caughtError: mixed = null; 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. @@ -50,7 +41,14 @@ export function invokeGuardedCallback( ): void { hasError = false; caughtError = null; - invokeGuardedCallbackImpl.apply(reporter, arguments); + try { + // $FlowFixMe[method-unbinding] + const funcArgs = Array.prototype.slice.call(arguments, 3); + func.apply(context, funcArgs); + } catch (error) { + hasError = true; + caughtError = error; + } } /** @@ -83,7 +81,14 @@ export function invokeGuardedCallbackAndCatchFirstError< e: E, f: F, ): void { - invokeGuardedCallback.apply(this, arguments); + try { + // $FlowFixMe[method-unbinding] + const funcArgs = Array.prototype.slice.call(arguments, 3); + func.apply(context, funcArgs); + } catch (error) { + hasError = true; + caughtError = error; + } if (hasError) { const error = clearCaughtError(); if (!hasRethrowError) { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 506f6746b98de..9604c3c5145a4 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 c78362e810e6a..715258772ac75 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -51,7 +51,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 1db47db31b857..8fa693e098b77 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 60e53b3fb3315..beff907b88dc6 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 5de35a8b21ee2..4003f0b0adec8 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 fbf88c8dbabac..baabd51eaac0c 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 39fc234de2ad6..5f340e1bbfe75 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -46,10 +46,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 4d77c65f4017e..fe0680db8b509 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/scripts/jest/shouldIgnoreConsoleError.js b/scripts/jest/shouldIgnoreConsoleError.js index 79ec7fc2ad7d6..c164043f96275 100644 --- a/scripts/jest/shouldIgnoreConsoleError.js +++ b/scripts/jest/shouldIgnoreConsoleError.js @@ -8,6 +8,7 @@ module.exports = function shouldIgnoreConsoleError( if (__DEV__) { if (typeof format === 'string') { if (format.indexOf('Error: Uncaught [') === 0) { + // TODO: Remove this. It shouldn't be needed anymore. // This looks like an uncaught error from invokeGuardedCallback() wrapper // in development that is reported by jsdom. Ignore because it's noisy. return true; @@ -43,6 +44,7 @@ module.exports = function shouldIgnoreConsoleError( args.length === 0 ) { if (format.stack.indexOf('Error: Uncaught [') === 0) { + // TODO: Remove this. It shouldn't be needed anymore. // 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; 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,