Land skipUnmountedBoundaries experiment (#23322)

This has been rolled out to 10% of Facebook users for months without any issues.
2026-09-24 17:02:27 +00:00 · 2022-02-17 21:43:25 -05:00
parent a0d98bdbc7
commit c7dd4ea40b
15 changed files with 4 additions and 50 deletions
@@ -42,7 +42,6 @@ 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;
@@ -30,7 +30,6 @@ import {
  enableSchedulingProfiler,
  disableSchedulerTimeoutInWorkLoop,
  enableStrictEffects,
-  skipUnmountedBoundaries,
  enableUpdaterTracking,
  warnOnSubscriptionInsideStartTransition,
  enableCache,
@@ -2445,13 +2444,7 @@ export function captureCommitPhaseError(
    return;
  }

-  let fiber = null;
-  if (skipUnmountedBoundaries) {
-    fiber = nearestMountedAncestor;
-  } else {
-    fiber = sourceFiber.return;
-  }
-
+  let fiber = nearestMountedAncestor;
  while (fiber !== null) {
    if (fiber.tag === HostRoot) {
      captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -2484,14 +2477,9 @@ 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. Likely ' +
+        'inside a detached tree. This indicates a bug in React. Potential ' +
        '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',
@@ -30,7 +30,6 @@ import {
  enableSchedulingProfiler,
  disableSchedulerTimeoutInWorkLoop,
  enableStrictEffects,
-  skipUnmountedBoundaries,
  enableUpdaterTracking,
  warnOnSubscriptionInsideStartTransition,
  enableCache,
@@ -2445,13 +2444,7 @@ export function captureCommitPhaseError(
    return;
  }

-  let fiber = null;
-  if (skipUnmountedBoundaries) {
-    fiber = nearestMountedAncestor;
-  } else {
-    fiber = sourceFiber.return;
-  }
-
+  let fiber = nearestMountedAncestor;
  while (fiber !== null) {
    if (fiber.tag === HostRoot) {
      captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -2484,14 +2477,9 @@ 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. Likely ' +
+        'inside a detached tree. This indicates a bug in React. Potential ' +
        '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',
@@ -2351,7 +2351,6 @@ describe('ReactHooksWithNoopRenderer', () => {
        };
      });

-      // @gate skipUnmountedBoundaries
      it('should use the nearest still-mounted boundary if there are no unmounted boundaries', () => {
        act(() => {
          ReactNoop.render(
@@ -2377,7 +2376,6 @@ describe('ReactHooksWithNoopRenderer', () => {
        ]);
      });

-      // @gate skipUnmountedBoundaries
      it('should skip unmounted boundaries and use the nearest still-mounted boundary', () => {
        function Conditional({showChildren}) {
          if (showChildren) {
@@ -2420,7 +2418,6 @@ describe('ReactHooksWithNoopRenderer', () => {
        ]);
      });

-      // @gate skipUnmountedBoundaries
      it('should call getDerivedStateFromError in the nearest still-mounted boundary', () => {
        function Conditional({showChildren}) {
          if (showChildren) {
@@ -2464,7 +2461,6 @@ describe('ReactHooksWithNoopRenderer', () => {
        ]);
      });

-      // @gate skipUnmountedBoundaries
      it('should rethrow error if there are no still-mounted boundaries', () => {
        function Conditional({showChildren}) {
          if (showChildren) {
@@ -3190,7 +3186,6 @@ describe('ReactHooksWithNoopRenderer', () => {
      ]);
    });

-    // @gate skipUnmountedBoundaries
    it('catches errors thrown in useLayoutEffect', () => {
      class ErrorBoundary extends React.Component {
        state = {error: null};
@@ -1017,7 +1017,6 @@ describe('ReactIncrementalErrorHandling', () => {
    expect(Scheduler).toFlushAndYield(['Foo']);
  });

-  // @gate skipUnmountedBoundaries
  it('should not attempt to recover an unmounting error boundary', () => {
    class Parent extends React.Component {
      componentWillUnmount() {
@@ -121,12 +121,6 @@ export const disableNativeComponentFrames = false;
 // Internal only.
 export const enableGetInspectorDataForInstanceInProduction = false;

-// Errors that are thrown while unmounting (or after in the case of passive effects)
-// should bypass any error boundaries that are also unmounting (or have unmounted)
-// and be handled by the nearest still-mounted boundary.
-// If there are no still-mounted boundaries, the errors should be rethrown.
-export const skipUnmountedBoundaries = false;
-
 // When a node is unmounted, recurse into the Fiber subtree and clean out
 // references. Each level cleans up more fiber fields than the previous level.
 // As far as we know, React itself doesn't leak, but because the Fiber contains
@@ -58,7 +58,6 @@ 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;
@@ -49,7 +49,6 @@ 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;
@@ -49,7 +49,6 @@ 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;
@@ -44,7 +44,6 @@ 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;
@@ -49,7 +49,6 @@ 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;
@@ -49,7 +49,6 @@ 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;
@@ -49,7 +49,6 @@ 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;
@@ -17,7 +17,6 @@ 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__;
@@ -24,7 +24,6 @@ export const {
  enableLegacyFBSupport,
  deferRenderPhaseUpdateToNextBatch,
  enableDebugTracing,
-  skipUnmountedBoundaries,
  createRootStrictEffectsByDefault,
  enableUseRefAccessWarning,
  disableNativeComponentFrames,