@samitouri / QOS-React / commits / 419ccc2b19

Land skipUnmountedBoundaries experiment (#23322)

This has been rolled out to 10% of Facebook users for months without any issues.

Andrew Clark committed Feb 17, 2022 at 21:43 UTC 419ccc2b1974206b9fcbe539a31654dd64024156
15 files changed +4 -50
packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
-1
@@ -42,7 +42,6 @@ describe('ReactErrorBoundaries', () => {
42 PropTypes = require('prop-types');
43 ReactFeatureFlags = require('shared/ReactFeatureFlags');
44 ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
45 - ReactFeatureFlags.skipUnmountedBoundaries = true;
45 ReactDOM = require('react-dom');
46 React = require('react');
47 act = require('jest-react').act;
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+2 -14
@@ -30,7 +30,6 @@ import {
30 enableSchedulingProfiler,
31 disableSchedulerTimeoutInWorkLoop,
32 enableStrictEffects,
33 - skipUnmountedBoundaries,
33 enableUpdaterTracking,
34 warnOnSubscriptionInsideStartTransition,
35 enableCache,
@@ -2445,13 +2444,7 @@ export function captureCommitPhaseError(
2444 return;
2445 }
2446
2448 - let fiber = null;
2449 - if (skipUnmountedBoundaries) {
2450 - fiber = nearestMountedAncestor;
2451 - } else {
2452 - fiber = sourceFiber.return;
2453 - }
2454 -
2447 + let fiber = nearestMountedAncestor;
2448 while (fiber !== null) {
2449 if (fiber.tag === HostRoot) {
2450 captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -2484,14 +2477,9 @@ export function captureCommitPhaseError(
2477 }
2478
2479 if (__DEV__) {
2487 - // TODO: Until we re-land skipUnmountedBoundaries (see #20147), this warning
2488 - // will fire for errors that are thrown by destroy functions inside deleted
2489 - // trees. What it should instead do is propagate the error to the parent of
2490 - // the deleted tree. In the meantime, do not add this warning to the
2491 - // allowlist; this is only for our internal use.
2480 console.error(
2481 'Internal React error: Attempted to capture a commit phase error ' +
2494 - 'inside a detached tree. This indicates a bug in React. Likely ' +
2482 + 'inside a detached tree. This indicates a bug in React. Potential ' +
2483 'causes include deleting the same fiber more than once, committing an ' +
2484 'already-finished tree, or an inconsistent return pointer.\n\n' +
2485 'Error message:\n\n%s',
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+2 -14
@@ -30,7 +30,6 @@ import {
30 enableSchedulingProfiler,
31 disableSchedulerTimeoutInWorkLoop,
32 enableStrictEffects,
33 - skipUnmountedBoundaries,
33 enableUpdaterTracking,
34 warnOnSubscriptionInsideStartTransition,
35 enableCache,
@@ -2445,13 +2444,7 @@ export function captureCommitPhaseError(
2444 return;
2445 }
2446
2448 - let fiber = null;
2449 - if (skipUnmountedBoundaries) {
2450 - fiber = nearestMountedAncestor;
2451 - } else {
2452 - fiber = sourceFiber.return;
2453 - }
2454 -
2447 + let fiber = nearestMountedAncestor;
2448 while (fiber !== null) {
2449 if (fiber.tag === HostRoot) {
2450 captureCommitPhaseErrorOnRoot(fiber, sourceFiber, error);
@@ -2484,14 +2477,9 @@ export function captureCommitPhaseError(
2477 }
2478
2479 if (__DEV__) {
2487 - // TODO: Until we re-land skipUnmountedBoundaries (see #20147), this warning
2488 - // will fire for errors that are thrown by destroy functions inside deleted
2489 - // trees. What it should instead do is propagate the error to the parent of
2490 - // the deleted tree. In the meantime, do not add this warning to the
2491 - // allowlist; this is only for our internal use.
2480 console.error(
2481 'Internal React error: Attempted to capture a commit phase error ' +
2494 - 'inside a detached tree. This indicates a bug in React. Likely ' +
2482 + 'inside a detached tree. This indicates a bug in React. Potential ' +
2483 'causes include deleting the same fiber more than once, committing an ' +
2484 'already-finished tree, or an inconsistent return pointer.\n\n' +
2485 'Error message:\n\n%s',
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
-5
@@ -2351,7 +2351,6 @@ describe('ReactHooksWithNoopRenderer', () => {
2351 };
2352 });
2353
2354 - // @gate skipUnmountedBoundaries
2354 it('should use the nearest still-mounted boundary if there are no unmounted boundaries', () => {
2355 act(() => {
2356 ReactNoop.render(
@@ -2377,7 +2376,6 @@ describe('ReactHooksWithNoopRenderer', () => {
2376 ]);
2377 });
2378
2380 - // @gate skipUnmountedBoundaries
2379 it('should skip unmounted boundaries and use the nearest still-mounted boundary', () => {
2380 function Conditional({showChildren}) {
2381 if (showChildren) {
@@ -2420,7 +2418,6 @@ describe('ReactHooksWithNoopRenderer', () => {
2418 ]);
2419 });
2420
2423 - // @gate skipUnmountedBoundaries
2421 it('should call getDerivedStateFromError in the nearest still-mounted boundary', () => {
2422 function Conditional({showChildren}) {
2423 if (showChildren) {
@@ -2464,7 +2461,6 @@ describe('ReactHooksWithNoopRenderer', () => {
2461 ]);
2462 });
2463
2467 - // @gate skipUnmountedBoundaries
2464 it('should rethrow error if there are no still-mounted boundaries', () => {
2465 function Conditional({showChildren}) {
2466 if (showChildren) {
@@ -3190,7 +3186,6 @@ describe('ReactHooksWithNoopRenderer', () => {
3186 ]);
3187 });
3188
3193 - // @gate skipUnmountedBoundaries
3189 it('catches errors thrown in useLayoutEffect', () => {
3190 class ErrorBoundary extends React.Component {
3191 state = {error: null};
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
-1
@@ -1017,7 +1017,6 @@ describe('ReactIncrementalErrorHandling', () => {
1017 expect(Scheduler).toFlushAndYield(['Foo']);
1018 });
1019
1020 - // @gate skipUnmountedBoundaries
1020 it('should not attempt to recover an unmounting error boundary', () => {
1021 class Parent extends React.Component {
1022 componentWillUnmount() {
packages/shared/ReactFeatureFlags.js
-6
@@ -121,12 +121,6 @@ export const disableNativeComponentFrames = false;
121 // Internal only.
122 export const enableGetInspectorDataForInstanceInProduction = false;
123
124 -// Errors that are thrown while unmounting (or after in the case of passive effects)
125 -// should bypass any error boundaries that are also unmounting (or have unmounted)
126 -// and be handled by the nearest still-mounted boundary.
127 -// If there are no still-mounted boundaries, the errors should be rethrown.
128 -export const skipUnmountedBoundaries = false;
129 -
124 // When a node is unmounted, recurse into the Fiber subtree and clean out
125 // references. Each level cleans up more fiber fields than the previous level.
126 // As far as we know, React itself doesn't leak, but because the Fiber contains
packages/shared/forks/ReactFeatureFlags.native-fb.js
-1
@@ -58,7 +58,6 @@ export const enableComponentStackLocations = false;
58 export const enableLegacyFBSupport = false;
59 export const enableFilterEmptyStringAttributesDOM = false;
60 export const disableNativeComponentFrames = false;
61 -export const skipUnmountedBoundaries = false;
61 export const deletedTreeCleanUpLevel = 3;
62 export const enableSuspenseLayoutEffectSemantics = false;
63 export const enableGetInspectorDataForInstanceInProduction = true;
packages/shared/forks/ReactFeatureFlags.native-oss.js
-1
@@ -49,7 +49,6 @@ export const enableComponentStackLocations = false;
49 export const enableLegacyFBSupport = false;
50 export const enableFilterEmptyStringAttributesDOM = false;
51 export const disableNativeComponentFrames = false;
52 -export const skipUnmountedBoundaries = false;
52 export const deletedTreeCleanUpLevel = 3;
53 export const enableSuspenseLayoutEffectSemantics = false;
54 export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
-1
@@ -49,7 +49,6 @@ export const enableComponentStackLocations = true;
49 export const enableLegacyFBSupport = false;
50 export const enableFilterEmptyStringAttributesDOM = false;
51 export const disableNativeComponentFrames = false;
52 -export const skipUnmountedBoundaries = false;
52 export const deletedTreeCleanUpLevel = 3;
53 export const enableSuspenseLayoutEffectSemantics = false;
54 export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
-1
@@ -44,7 +44,6 @@ export const enableComponentStackLocations = false;
44 export const enableLegacyFBSupport = false;
45 export const enableFilterEmptyStringAttributesDOM = false;
46 export const disableNativeComponentFrames = false;
47 -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
@@ -49,7 +49,6 @@ export const enableComponentStackLocations = true;
49 export const enableLegacyFBSupport = false;
50 export const enableFilterEmptyStringAttributesDOM = false;
51 export const disableNativeComponentFrames = false;
52 -export const skipUnmountedBoundaries = false;
52 export const deletedTreeCleanUpLevel = 3;
53 export const enableSuspenseLayoutEffectSemantics = false;
54 export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.testing.js
-1
@@ -49,7 +49,6 @@ export const enableComponentStackLocations = true;
49 export const enableLegacyFBSupport = false;
50 export const enableFilterEmptyStringAttributesDOM = false;
51 export const disableNativeComponentFrames = false;
52 -export const skipUnmountedBoundaries = false;
52 export const deletedTreeCleanUpLevel = 3;
53 export const enableSuspenseLayoutEffectSemantics = false;
54 export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
-1
@@ -49,7 +49,6 @@ export const enableComponentStackLocations = true;
49 export const enableLegacyFBSupport = !__EXPERIMENTAL__;
50 export const enableFilterEmptyStringAttributesDOM = false;
51 export const disableNativeComponentFrames = false;
52 -export const skipUnmountedBoundaries = true;
52 export const deletedTreeCleanUpLevel = 3;
53 export const enableSuspenseLayoutEffectSemantics = false;
54 export const enableGetInspectorDataForInstanceInProduction = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
-1
@@ -17,7 +17,6 @@ export const warnAboutSpreadingKeyToJSX = __VARIANT__;
17 export const disableInputAttributeSyncing = __VARIANT__;
18 export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
19 export const enableLegacyFBSupport = __VARIANT__;
20 -export const skipUnmountedBoundaries = __VARIANT__;
20 export const enableUseRefAccessWarning = __VARIANT__;
21 export const deletedTreeCleanUpLevel = __VARIANT__ ? 3 : 1;
22 export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.www.js
-1
@@ -24,7 +24,6 @@ export const {
24 enableLegacyFBSupport,
25 deferRenderPhaseUpdateToNextBatch,
26 enableDebugTracing,
27 - skipUnmountedBoundaries,
27 createRootStrictEffectsByDefault,
28 enableUseRefAccessWarning,
29 disableNativeComponentFrames,