From d73d7d59086218b0fa42d0a79c32a0365952650b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 20 Apr 2023 14:23:22 -0400 Subject: [PATCH] Add `alwaysThrottleRetries` flag (#26685) This puts the change introduced by #26611 behind a flag until Meta is able to roll it out. Disabling the flag reverts back to the old behavior, where retries are throttled if there's still data remaining in the tree, but not if all the data has finished loading. The new behavior is still enabled in the public builds. --- .../src/ReactFiberWorkLoop.js | 6 ++++- .../ReactSuspenseWithNoopRenderer-test.js | 24 ++++++++++++++++--- packages/shared/ReactFeatureFlags.js | 2 ++ .../ReactFeatureFlags.native-fb-dynamic.js | 1 + .../forks/ReactFeatureFlags.native-fb.js | 7 ++++-- .../forks/ReactFeatureFlags.native-oss.js | 2 ++ .../forks/ReactFeatureFlags.test-renderer.js | 2 ++ .../ReactFeatureFlags.test-renderer.native.js | 2 ++ .../ReactFeatureFlags.test-renderer.www.js | 2 ++ .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/flow/xplat.js | 1 + 12 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 96c4ea9255..e6764086ea 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -39,6 +39,7 @@ import { enableTransitionTracing, useModernStrictMode, disableLegacyContext, + alwaysThrottleRetries, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; @@ -1115,7 +1116,10 @@ function finishConcurrentRender( workInProgressTransitions, ); } else { - if (includesOnlyRetries(lanes)) { + if ( + includesOnlyRetries(lanes) && + (alwaysThrottleRetries || exitStatus === RootSuspended) + ) { // This render only included retries, no updates. Throttle committing // retries so that we don't show too many loading states too quickly. const msUntilTimeout = diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 2171046831..e6019ffd40 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1779,10 +1779,28 @@ describe('ReactSuspenseWithNoopRenderer', () => { await resolveText('B'); expect(ReactNoop).toMatchRenderedOutput(); - // Restart and render the complete content. The tree will finish but we - // won't commit the result yet because the fallback appeared recently. + // Restart and render the complete content. await waitForAll(['A', 'B']); - expect(ReactNoop).toMatchRenderedOutput(); + + if (gate(flags => flags.alwaysThrottleRetries)) { + // Correct behavior: + // + // The tree will finish but we won't commit the result yet because the fallback appeared recently. + expect(ReactNoop).toMatchRenderedOutput(); + } else { + // Old behavior, gated until this rolls out at Meta: + // + // TODO: Because this render was the result of a retry, and a fallback + // was shown recently, we should suspend and remain on the fallback for + // little bit longer. We currently only do this if there's still + // remaining fallbacks in the tree, but we should do it for all retries. + expect(ReactNoop).toMatchRenderedOutput( + <> + + + , + ); + } }); assertLog([]); expect(ReactNoop).toMatchRenderedOutput( diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 83350f83ee..6129bb1476 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -124,6 +124,8 @@ export const diffInCommitPhase = __EXPERIMENTAL__; export const enableAsyncActions = __EXPERIMENTAL__; +export const alwaysThrottleRetries = true; + // ----------------------------------------------------------------------------- // Chopping Block // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index 2270b18ae3..92449bcd6a 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -22,6 +22,7 @@ import typeof * as DynamicFlagsType from 'ReactNativeInternalFeatureFlags'; export const enableUseRefAccessWarning = __VARIANT__; export const enableDeferRootSchedulingToMicrotask = __VARIANT__; +export const alwaysThrottleRetries = __VARIANT__; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): DynamicFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 370207d9d0..9345b0bb85 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -17,8 +17,11 @@ import * as dynamicFlags from 'ReactNativeInternalFeatureFlags'; // We destructure each value before re-exporting to avoid a dynamic look-up on // the exports object every time a flag is read. -export const {enableUseRefAccessWarning, enableDeferRootSchedulingToMicrotask} = - dynamicFlags; +export const { + enableUseRefAccessWarning, + enableDeferRootSchedulingToMicrotask, + alwaysThrottleRetries, +} = dynamicFlags; // The rest of the flags are static for better dead code elimination. export const enableDebugTracing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 1b9aa46310..c5888b2837 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -75,5 +75,7 @@ export const enableDeferRootSchedulingToMicrotask = true; export const diffInCommitPhase = true; export const enableAsyncActions = false; +export const alwaysThrottleRetries = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index c2e8fd14f5..63f3c9f6eb 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -75,5 +75,7 @@ export const enableDeferRootSchedulingToMicrotask = true; export const diffInCommitPhase = true; export const enableAsyncActions = false; +export const alwaysThrottleRetries = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index ce6fe31f10..771362d6bf 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -72,5 +72,7 @@ export const enableDeferRootSchedulingToMicrotask = true; export const diffInCommitPhase = true; export const enableAsyncActions = false; +export const alwaysThrottleRetries = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 4fa45a9142..433d18d918 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -77,5 +77,7 @@ export const enableDeferRootSchedulingToMicrotask = true; export const diffInCommitPhase = true; export const enableAsyncActions = false; +export const alwaysThrottleRetries = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index ad284d30c5..827d1413b5 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -27,6 +27,7 @@ export const enableCustomElementPropertySupport = __VARIANT__; export const enableDeferRootSchedulingToMicrotask = __VARIANT__; export const diffInCommitPhase = __VARIANT__; export const enableAsyncActions = __VARIANT__; +export const alwaysThrottleRetries = __VARIANT__; // Enable this flag to help with concurrent mode debugging. // It logs information to the console about React scheduling, rendering, and commit phases. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index e578b08dd8..a4a64c0d93 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -30,6 +30,7 @@ export const { enableDeferRootSchedulingToMicrotask, diffInCommitPhase, enableAsyncActions, + alwaysThrottleRetries, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. diff --git a/scripts/flow/xplat.js b/scripts/flow/xplat.js index 8e14cf5a04..37d1deee75 100644 --- a/scripts/flow/xplat.js +++ b/scripts/flow/xplat.js @@ -10,4 +10,5 @@ declare module 'ReactNativeInternalFeatureFlags' { declare export var enableUseRefAccessWarning: boolean; declare export var enableDeferRootSchedulingToMicrotask: boolean; + declare export var alwaysThrottleRetries: boolean; }