Add back skipUnmountedBoundaries flag only for www (#23383)
There are a few internal tests that still need to be updated, so I'm adding this flag back for www only. The desired behavior rolled out to 10% public, so we're confident there are no issues. The open source behavior remains (skipUnmountedBoundaries = true).
Andrew Clark committed
Feb 28, 2022 at 11:14 UTC
a82ef6d40bb0b000a97ca82590bfeaa3637c66bb
15 files changed
+48
-4
packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
+1
@@ -42,6 +42,7 @@ describe('ReactErrorBoundaries', () => {
42
PropTypes = require('prop-types');
43
ReactFeatureFlags = require('shared/ReactFeatureFlags');
44
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
45
+ ReactFeatureFlags.skipUnmountedBoundaries = true;
46
ReactDOM = require('react-dom');
47
React = require('react');
48
act = require('jest-react').act;
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+14
-2
@@ -35,6 +35,7 @@ import {
35
enableSchedulingProfiler,
36
disableSchedulerTimeoutInWorkLoop,
37
enableStrictEffects,
38
+ skipUnmountedBoundaries,
39
enableUpdaterTracking,
40
enableCache,
41
enableTransitionTracing,
@@ -2537,7 +2538,13 @@ export function captureCommitPhaseError(
2538
return;
2539
}
2540
2540
- let fiber = nearestMountedAncestor;
2541
+ let fiber = null;
2542
+ if (skipUnmountedBoundaries) {
2543
+ fiber = nearestMountedAncestor;
2544
+ } else {
2545
+ fiber = sourceFiber.return;
2546
+ }
2547
+
2548
while (fiber !== null) {
2549
if (fiber.tag === HostRoot) {
2550
captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -2570,9 +2577,14 @@ export function captureCommitPhaseError(
2577
}
2578
2579
if (__DEV__) {
2580
+ // TODO: Until we re-land skipUnmountedBoundaries (see #20147), this warning
2581
+ // will fire for errors that are thrown by destroy functions inside deleted
2582
+ // trees. What it should instead do is propagate the error to the parent of
2583
+ // the deleted tree. In the meantime, do not add this warning to the
2584
+ // allowlist; this is only for our internal use.
2585
console.error(
2586
'Internal React error: Attempted to capture a commit phase error ' +
2575
- 'inside a detached tree. This indicates a bug in React. Potential ' +
2587
+ 'inside a detached tree. This indicates a bug in React. Likely ' +
2588
'causes include deleting the same fiber more than once, committing an ' +
2589
'already-finished tree, or an inconsistent return pointer.\n\n' +
2590
'Error message:\n\n%s',
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+14
-2
@@ -35,6 +35,7 @@ import {
35
enableSchedulingProfiler,
36
disableSchedulerTimeoutInWorkLoop,
37
enableStrictEffects,
38
+ skipUnmountedBoundaries,
39
enableUpdaterTracking,
40
enableCache,
41
enableTransitionTracing,
@@ -2537,7 +2538,13 @@ export function captureCommitPhaseError(
2538
return;
2539
}
2540
2540
- let fiber = nearestMountedAncestor;
2541
+ let fiber = null;
2542
+ if (skipUnmountedBoundaries) {
2543
+ fiber = nearestMountedAncestor;
2544
+ } else {
2545
+ fiber = sourceFiber.return;
2546
+ }
2547
+
2548
while (fiber !== null) {
2549
if (fiber.tag === HostRoot) {
2550
captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -2570,9 +2577,14 @@ export function captureCommitPhaseError(
2577
}
2578
2579
if (__DEV__) {
2580
+ // TODO: Until we re-land skipUnmountedBoundaries (see #20147), this warning
2581
+ // will fire for errors that are thrown by destroy functions inside deleted
2582
+ // trees. What it should instead do is propagate the error to the parent of
2583
+ // the deleted tree. In the meantime, do not add this warning to the
2584
+ // allowlist; this is only for our internal use.
2585
console.error(
2586
'Internal React error: Attempted to capture a commit phase error ' +
2575
- 'inside a detached tree. This indicates a bug in React. Potential ' +
2587
+ 'inside a detached tree. This indicates a bug in React. Likely ' +
2588
'causes include deleting the same fiber more than once, committing an ' +
2589
'already-finished tree, or an inconsistent return pointer.\n\n' +
2590
'Error message:\n\n%s',
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
+5
@@ -2351,6 +2351,7 @@ describe('ReactHooksWithNoopRenderer', () => {
2351
};
2352
});
2353
2354
+ // @gate skipUnmountedBoundaries
2355
it('should use the nearest still-mounted boundary if there are no unmounted boundaries', () => {
2356
act(() => {
2357
ReactNoop.render(
@@ -2376,6 +2377,7 @@ describe('ReactHooksWithNoopRenderer', () => {
2377
]);
2378
});
2379
2380
+ // @gate skipUnmountedBoundaries
2381
it('should skip unmounted boundaries and use the nearest still-mounted boundary', () => {
2382
function Conditional({showChildren}) {
2383
if (showChildren) {
@@ -2418,6 +2420,7 @@ describe('ReactHooksWithNoopRenderer', () => {
2420
]);
2421
});
2422
2423
+ // @gate skipUnmountedBoundaries
2424
it('should call getDerivedStateFromError in the nearest still-mounted boundary', () => {
2425
function Conditional({showChildren}) {
2426
if (showChildren) {
@@ -2461,6 +2464,7 @@ describe('ReactHooksWithNoopRenderer', () => {
2464
]);
2465
});
2466
2467
+ // @gate skipUnmountedBoundaries
2468
it('should rethrow error if there are no still-mounted boundaries', () => {
2469
function Conditional({showChildren}) {
2470
if (showChildren) {
@@ -3186,6 +3190,7 @@ describe('ReactHooksWithNoopRenderer', () => {
3190
]);
3191
});
3192
3193
+ // @gate skipUnmountedBoundaries
3194
it('catches errors thrown in useLayoutEffect', () => {
3195
class ErrorBoundary extends React.Component {
3196
state = {error: null};
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
+1
@@ -1017,6 +1017,7 @@ describe('ReactIncrementalErrorHandling', () => {
1017
expect(Scheduler).toFlushAndYield(['Foo']);
1018
});
1019
1020
+ // @gate skipUnmountedBoundaries
1021
it('should not attempt to recover an unmounting error boundary', () => {
1022
class Parent extends React.Component {
1023
componentWillUnmount() {
packages/shared/ReactFeatureFlags.js
+4
@@ -28,6 +28,10 @@ export const enablePersistentOffscreenHostContainer = false;
28
// like migrating internal callers or performance testing.
29
// -----------------------------------------------------------------------------
30
31
+// This rolled out to 10% public in www, so we should be able to land, but some
32
+// internal tests need to be updated. The open source behavior is correct.
33
+export const skipUnmountedBoundaries = true;
34
+
35
// Destroy layout effects for components that are hidden because something
36
// suspended in an update and recreate them when they are shown again (after the
37
// suspended boundary has resolved). Note that this should be an uncommon use
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -56,6 +56,7 @@ export const enableComponentStackLocations = false;
56
export const enableLegacyFBSupport = false;
57
export const enableFilterEmptyStringAttributesDOM = false;
58
export const disableNativeComponentFrames = false;
59
+export const skipUnmountedBoundaries = false;
60
export const deletedTreeCleanUpLevel = 3;
61
export const enableSuspenseLayoutEffectSemantics = false;
62
export const enableGetInspectorDataForInstanceInProduction = true;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -47,6 +47,7 @@ export const enableComponentStackLocations = false;
47
export const enableLegacyFBSupport = false;
48
export const enableFilterEmptyStringAttributesDOM = false;
49
export const disableNativeComponentFrames = false;
50
+export const skipUnmountedBoundaries = false;
51
export const deletedTreeCleanUpLevel = 3;
52
export const enableSuspenseLayoutEffectSemantics = false;
53
export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -47,6 +47,7 @@ export const enableComponentStackLocations = true;
47
export const enableLegacyFBSupport = false;
48
export const enableFilterEmptyStringAttributesDOM = false;
49
export const disableNativeComponentFrames = false;
50
+export const skipUnmountedBoundaries = false;
51
export const deletedTreeCleanUpLevel = 3;
52
export const enableSuspenseLayoutEffectSemantics = false;
53
export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
@@ -43,6 +43,7 @@ export const enableComponentStackLocations = false;
43
export const enableLegacyFBSupport = false;
44
export const enableFilterEmptyStringAttributesDOM = false;
45
export const disableNativeComponentFrames = false;
46
+export const skipUnmountedBoundaries = false;
47
export const deletedTreeCleanUpLevel = 3;
48
export const enableSuspenseLayoutEffectSemantics = false;
49
export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -47,6 +47,7 @@ export const enableComponentStackLocations = true;
47
export const enableLegacyFBSupport = false;
48
export const enableFilterEmptyStringAttributesDOM = false;
49
export const disableNativeComponentFrames = false;
50
+export const skipUnmountedBoundaries = false;
51
export const deletedTreeCleanUpLevel = 3;
52
export const enableSuspenseLayoutEffectSemantics = false;
53
export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -47,6 +47,7 @@ export const enableComponentStackLocations = true;
47
export const enableLegacyFBSupport = false;
48
export const enableFilterEmptyStringAttributesDOM = false;
49
export const disableNativeComponentFrames = false;
50
+export const skipUnmountedBoundaries = false;
51
export const deletedTreeCleanUpLevel = 3;
52
export const enableSuspenseLayoutEffectSemantics = false;
53
export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -47,6 +47,7 @@ export const enableComponentStackLocations = true;
47
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
48
export const enableFilterEmptyStringAttributesDOM = false;
49
export const disableNativeComponentFrames = false;
50
+export const skipUnmountedBoundaries = true;
51
export const deletedTreeCleanUpLevel = 3;
52
export const enableSuspenseLayoutEffectSemantics = false;
53
export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -17,6 +17,7 @@ export const warnAboutSpreadingKeyToJSX = __VARIANT__;
17
export const disableInputAttributeSyncing = __VARIANT__;
18
export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
19
export const enableLegacyFBSupport = __VARIANT__;
20
+export const skipUnmountedBoundaries = __VARIANT__;
21
export const enableUseRefAccessWarning = __VARIANT__;
22
export const deletedTreeCleanUpLevel = __VARIANT__ ? 3 : 1;
23
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -24,6 +24,7 @@ export const {
24
enableLegacyFBSupport,
25
deferRenderPhaseUpdateToNextBatch,
26
enableDebugTracing,
27
+ skipUnmountedBoundaries,
28
createRootStrictEffectsByDefault,
29
enableUseRefAccessWarning,
30
disableNativeComponentFrames,