Remove skipUnmountedBoundaries (#26489)

rickhanlonii · web-flow · commit ca01f359b923 · 2023-03-30T20:58:13.000-04:00
# Overview

Landing this flag internally, will test this PR in React Native before
merging.
diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
@@ -43,7 +43,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('internal-test-utils').act;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -33,7 +33,6 @@ import {
   enableDebugTracing,
   enableSchedulingProfiler,
   disableSchedulerTimeoutInWorkLoop,
-  skipUnmountedBoundaries,
   enableUpdaterTracking,
   enableCache,
   enableTransitionTracing,
@@ -3517,13 +3516,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);
@@ -3555,14 +3548,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',
diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
@@ -2254,7 +2254,6 @@ describe('ReactHooksWithNoopRenderer', () => {
         };
       });
 
-      // @gate skipUnmountedBoundaries
       it('should use the nearest still-mounted boundary if there are no unmounted boundaries', async () => {
         await act(() => {
           ReactNoop.render(
@@ -2280,7 +2279,6 @@ describe('ReactHooksWithNoopRenderer', () => {
         ]);
       });
 
-      // @gate skipUnmountedBoundaries
       it('should skip unmounted boundaries and use the nearest still-mounted boundary', async () => {
         function Conditional({showChildren}) {
           if (showChildren) {
@@ -2323,7 +2321,6 @@ describe('ReactHooksWithNoopRenderer', () => {
         ]);
       });
 
-      // @gate skipUnmountedBoundaries
       it('should call getDerivedStateFromError in the nearest still-mounted boundary', async () => {
         function Conditional({showChildren}) {
           if (showChildren) {
@@ -2367,7 +2364,6 @@ describe('ReactHooksWithNoopRenderer', () => {
         );
       });
 
-      // @gate skipUnmountedBoundaries
       it('should rethrow error if there are no still-mounted boundaries', async () => {
         function Conditional({showChildren}) {
           if (showChildren) {
@@ -2531,10 +2527,6 @@ describe('ReactHooksWithNoopRenderer', () => {
       assertLog(['layout destroy', 'passive destroy']);
     });
 
-    // TODO: This test fails when skipUnmountedBoundaries is disabled. However,
-    // it's also rolled out to open source already and partially to www. So
-    // we should probably just land it.
-    // @gate skipUnmountedBoundaries
     it('assumes passive effect destroy function is either a function or undefined', async () => {
       function App(props) {
         useEffect(() => {
@@ -3117,7 +3109,6 @@ describe('ReactHooksWithNoopRenderer', () => {
       assertLog(['Unmount normal [current: 1]', 'Mount normal [current: 1]']);
     });
 
-    // @gate skipUnmountedBoundaries
     it('catches errors thrown in useLayoutEffect', async () => {
       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
@@ -962,7 +962,6 @@ describe('ReactIncrementalErrorHandling', () => {
     await waitForAll(['Foo']);
   });
 
-  // @gate skipUnmountedBoundaries
   it('should not attempt to recover an unmounting error boundary', async () => {
     class Parent extends React.Component {
       componentWillUnmount() {
diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -34,10 +34,6 @@ export const revertRemovalOfSiblingPrerendering = 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;
-
 // TODO: Finish rolling out in www
 export const enableClientRenderFallbackOnTextMismatch = true;
 
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -58,7 +58,6 @@ export const enableClientRenderFallbackOnTextMismatch = true;
 export const enableComponentStackLocations = false;
 export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
-export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = true;
 
 export const createRootStrictEffectsByDefault = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -48,7 +48,6 @@ export const enableClientRenderFallbackOnTextMismatch = true;
 export const enableComponentStackLocations = false;
 export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
-export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
 
 export const createRootStrictEffectsByDefault = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -48,7 +48,6 @@ export const enableClientRenderFallbackOnTextMismatch = true;
 export const enableComponentStackLocations = true;
 export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
-export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
 
 export const createRootStrictEffectsByDefault = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
@@ -39,7 +39,6 @@ export const disableModulePatternComponents = false;
 export const enableComponentStackLocations = false;
 export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = false;
-export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
 export const enableSuspenseAvoidThisFallback = false;
 export const enableSuspenseAvoidThisFallbackFizz = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -48,7 +48,6 @@ export const enableClientRenderFallbackOnTextMismatch = true;
 export const enableComponentStackLocations = true;
 export const enableLegacyFBSupport = false;
 export const enableFilterEmptyStringAttributesDOM = true;
-export const skipUnmountedBoundaries = false;
 export const enableGetInspectorDataForInstanceInProduction = false;
 
 export const createRootStrictEffectsByDefault = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js
@@ -16,7 +16,6 @@
 export const disableInputAttributeSyncing = __VARIANT__;
 export const disableIEWorkarounds = __VARIANT__;
 export const enableLegacyFBSupport = __VARIANT__;
-export const skipUnmountedBoundaries = __VARIANT__;
 export const enableUseRefAccessWarning = __VARIANT__;
 export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
 export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -22,7 +22,6 @@ export const {
   replayFailedUnitOfWorkWithInvokeGuardedCallback,
   enableLegacyFBSupport,
   enableDebugTracing,
-  skipUnmountedBoundaries,
   enableUseRefAccessWarning,
   enableLazyContextPropagation,
   enableUnifiedSyncLane,
