diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 8b1936cc8e11..c5a1403ddc52 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -42,6 +42,7 @@ describe('ReactErrorBoundaries', () => { PropTypes = require('prop-types'); ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; + ReactFeatureFlags.skipUnmountedBoundaries = true; ReactDOM = require('react-dom'); React = require('react'); act = require('jest-react').act; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index a529454dcd37..7223ad7d052b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -35,6 +35,7 @@ import { enableSchedulingProfiler, disableSchedulerTimeoutInWorkLoop, enableStrictEffects, + skipUnmountedBoundaries, enableUpdaterTracking, enableCache, enableTransitionTracing, @@ -2537,7 +2538,13 @@ export function captureCommitPhaseError( return; } - let fiber = nearestMountedAncestor; + let fiber = null; + if (skipUnmountedBoundaries) { + fiber = nearestMountedAncestor; + } else { + fiber = sourceFiber.return; + } + while (fiber !== null) { if (fiber.tag === HostRoot) { captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error); @@ -2570,9 +2577,14 @@ export function captureCommitPhaseError( } if (__DEV__) { + // TODO: Until we re-land skipUnmountedBoundaries (see #20147), this warning + // will fire for errors that are thrown by destroy functions inside deleted + // trees. What it should instead do is propagate the error to the parent of + // the deleted tree. In the meantime, do not add this warning to the + // allowlist; this is only for our internal use. console.error( 'Internal React error: Attempted to capture a commit phase error ' + - 'inside a detached tree. This indicates a bug in React. Potential ' + + 'inside a detached tree. This indicates a bug in React. Likely ' + 'causes include deleting the same fiber more than once, committing an ' + 'already-finished tree, or an inconsistent return pointer.\n\n' + 'Error message:\n\n%s', diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 86ad6216d128..d8bb6b16e29f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -35,6 +35,7 @@ import { enableSchedulingProfiler, disableSchedulerTimeoutInWorkLoop, enableStrictEffects, + skipUnmountedBoundaries, enableUpdaterTracking, enableCache, enableTransitionTracing, @@ -2537,7 +2538,13 @@ export function captureCommitPhaseError( return; } - let fiber = nearestMountedAncestor; + let fiber = null; + if (skipUnmountedBoundaries) { + fiber = nearestMountedAncestor; + } else { + fiber = sourceFiber.return; + } + while (fiber !== null) { if (fiber.tag === HostRoot) { captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error); @@ -2570,9 +2577,14 @@ export function captureCommitPhaseError( } if (__DEV__) { + // TODO: Until we re-land skipUnmountedBoundaries (see #20147), this warning + // will fire for errors that are thrown by destroy functions inside deleted + // trees. What it should instead do is propagate the error to the parent of + // the deleted tree. In the meantime, do not add this warning to the + // allowlist; this is only for our internal use. console.error( 'Internal React error: Attempted to capture a commit phase error ' + - 'inside a detached tree. This indicates a bug in React. Potential ' + + 'inside a detached tree. This indicates a bug in React. Likely ' + 'causes include deleting the same fiber more than once, committing an ' + 'already-finished tree, or an inconsistent return pointer.\n\n' + 'Error message:\n\n%s', diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 97fffdcbd480..811be0963915 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2351,6 +2351,7 @@ describe('ReactHooksWithNoopRenderer', () => { }; }); + // @gate skipUnmountedBoundaries it('should use the nearest still-mounted boundary if there are no unmounted boundaries', () => { act(() => { ReactNoop.render( @@ -2376,6 +2377,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); }); + // @gate skipUnmountedBoundaries it('should skip unmounted boundaries and use the nearest still-mounted boundary', () => { function Conditional({showChildren}) { if (showChildren) { @@ -2418,6 +2420,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); }); + // @gate skipUnmountedBoundaries it('should call getDerivedStateFromError in the nearest still-mounted boundary', () => { function Conditional({showChildren}) { if (showChildren) { @@ -2461,6 +2464,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); }); + // @gate skipUnmountedBoundaries it('should rethrow error if there are no still-mounted boundaries', () => { function Conditional({showChildren}) { if (showChildren) { @@ -3186,6 +3190,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); }); + // @gate skipUnmountedBoundaries it('catches errors thrown in useLayoutEffect', () => { class ErrorBoundary extends React.Component { state = {error: null}; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 3aa27ceaba69..55ce08b45450 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1017,6 +1017,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(Scheduler).toFlushAndYield(['Foo']); }); + // @gate skipUnmountedBoundaries it('should not attempt to recover an unmounting error boundary', () => { class Parent extends React.Component { componentWillUnmount() { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index ba8a3bd437cc..d33d2a92d29c 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -28,6 +28,10 @@ export const enablePersistentOffscreenHostContainer = false; // like migrating internal callers or performance testing. // ----------------------------------------------------------------------------- +// This rolled out to 10% public in www, so we should be able to land, but some +// internal tests need to be updated. The open source behavior is correct. +export const skipUnmountedBoundaries = true; + // Destroy layout effects for components that are hidden because something // suspended in an update and recreate them when they are shown again (after the // suspended boundary has resolved). Note that this should be an uncommon use diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 5bf0f37db9eb..1eb2086c896b 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -56,6 +56,7 @@ export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = false; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 5c97146a82b2..e9850c400a9a 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -47,6 +47,7 @@ export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = false; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index b44bc97206d0..cd488a2b7e5d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -47,6 +47,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = false; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 4b8c1b774179..54828d7ebd6a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -43,6 +43,7 @@ export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = false; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index b6f620e2fb70..6d1dcfa842cc 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -47,6 +47,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = false; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index dbfc8fb70453..e6c46ee341f0 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -47,6 +47,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = false; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 95fa25cf592c..0ae82026016a 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -47,6 +47,7 @@ export const enableComponentStackLocations = true; export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; +export const skipUnmountedBoundaries = true; export const deletedTreeCleanUpLevel = 3; export const enableSuspenseLayoutEffectSemantics = false; export const enableGetInspectorDataForInstanceInProduction = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 36627a5d50d3..173713b36cf1 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -17,6 +17,7 @@ export const warnAboutSpreadingKeyToJSX = __VARIANT__; export const disableInputAttributeSyncing = __VARIANT__; export const enableFilterEmptyStringAttributesDOM = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__; +export const skipUnmountedBoundaries = __VARIANT__; export const enableUseRefAccessWarning = __VARIANT__; export const deletedTreeCleanUpLevel = __VARIANT__ ? 3 : 1; export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index ac50fe4fd9f3..cf61f17054d4 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -24,6 +24,7 @@ export const { enableLegacyFBSupport, deferRenderPhaseUpdateToNextBatch, enableDebugTracing, + skipUnmountedBoundaries, createRootStrictEffectsByDefault, enableUseRefAccessWarning, disableNativeComponentFrames,