Skip to content

Commit

Permalink
Remove skipUnmountedBoundaries (#26489)
Browse files Browse the repository at this point in the history
# Overview

Landing this flag internally, will test this PR in React Native before
merging.
  • Loading branch information
rickhanlonii authored Mar 31, 2023
1 parent 43a70a6 commit ca01f35
Show file tree
Hide file tree
Showing 12 changed files with 2 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 2 additions & 14 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
enableDebugTracing,
enableSchedulingProfiler,
disableSchedulerTimeoutInWorkLoop,
skipUnmountedBoundaries,
enableUpdaterTracking,
enableCache,
enableTransitionTracing,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) {
Expand Down Expand Up @@ -2323,7 +2321,6 @@ describe('ReactHooksWithNoopRenderer', () => {
]);
});

// @gate skipUnmountedBoundaries
it('should call getDerivedStateFromError in the nearest still-mounted boundary', async () => {
function Conditional({showChildren}) {
if (showChildren) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 0 additions & 4 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const {
replayFailedUnitOfWorkWithInvokeGuardedCallback,
enableLegacyFBSupport,
enableDebugTracing,
skipUnmountedBoundaries,
enableUseRefAccessWarning,
enableLazyContextPropagation,
enableUnifiedSyncLane,
Expand Down

0 comments on commit ca01f35

Please sign in to comment.