Rendering tasks should not jump the queue (#16284)
When React schedules a rendering task, it passes a `timeout` option based on its expiration time. This is intended to avoid starvation by other React updates. However, it also affects the relative priority of React tasks and other Scheduler tasks at the same level, like data processing. This adds a feature flag to disable passing a `timeout` option to Scheduler. React tasks will always append themselves to the end of the queue, without jumping ahead of already scheduled tasks. This does not affect the order in which React updates within a single root are processed, but it could affect updates across multiple roots. This also doesn't remove the expiration from Scheduler. It only means that React tasks are not given special treatment.
Andrew Clark committed
Aug 2, 2019 at 17:52 UTC
6b565ce736657308393e75d507b7c374e1f2bc8c
9 files changed
+109
-1
packages/react-reconciler/src/ReactFiberWorkLoop.js
+5
-1
@@ -27,6 +27,7 @@ import {
27
revertPassiveEffectsChange,
28
warnAboutUnmockedScheduler,
29
flushSuspenseFallbacksInTests,
30
+ disableSchedulerTimeoutBasedOnReactExpirationTime,
31
} from 'shared/ReactFeatureFlags';
32
import ReactSharedInternals from 'shared/ReactSharedInternals';
33
import invariant from 'shared/invariant';
@@ -530,7 +531,10 @@ function scheduleCallbackForRoot(
531
);
532
} else {
533
let options = null;
533
- if (expirationTime !== Never) {
534
+ if (
535
+ !disableSchedulerTimeoutBasedOnReactExpirationTime &&
536
+ expirationTime !== Never
537
+ ) {
538
let timeout = expirationTimeToMs(expirationTime) - now();
539
options = {timeout};
540
}
packages/react-reconciler/src/__tests__/ReactDisableSchedulerTimeoutBasedOnReactExpirationTime-test.internal.js
new
+96
@@ -0,0 +1,96 @@
1
+let React;
2
+let ReactFeatureFlags;
3
+let ReactNoop;
4
+let Scheduler;
5
+let Suspense;
6
+let scheduleCallback;
7
+let NormalPriority;
8
+
9
+describe('ReactSuspenseList', () => {
10
+ beforeEach(() => {
11
+ jest.resetModules();
12
+ ReactFeatureFlags = require('shared/ReactFeatureFlags');
13
+ ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
14
+ ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
15
+ ReactFeatureFlags.disableSchedulerTimeoutBasedOnReactExpirationTime = true;
16
+ React = require('react');
17
+ ReactNoop = require('react-noop-renderer');
18
+ Scheduler = require('scheduler');
19
+ Suspense = React.Suspense;
20
+
21
+ scheduleCallback = Scheduler.unstable_scheduleCallback;
22
+ NormalPriority = Scheduler.unstable_NormalPriority;
23
+ });
24
+
25
+ function Text(props) {
26
+ Scheduler.unstable_yieldValue(props.text);
27
+ return props.text;
28
+ }
29
+
30
+ function createAsyncText(text) {
31
+ let resolved = false;
32
+ let Component = function() {
33
+ if (!resolved) {
34
+ Scheduler.unstable_yieldValue('Suspend! [' + text + ']');
35
+ throw promise;
36
+ }
37
+ return <Text text={text} />;
38
+ };
39
+ let promise = new Promise(resolve => {
40
+ Component.resolve = function() {
41
+ resolved = true;
42
+ return resolve();
43
+ };
44
+ });
45
+ return Component;
46
+ }
47
+
48
+ it('appends rendering tasks to the end of the priority queue', async () => {
49
+ const A = createAsyncText('A');
50
+ const B = createAsyncText('B');
51
+
52
+ function App({show}) {
53
+ return (
54
+ <Suspense fallback={<Text text="Loading..." />}>
55
+ {show ? <A /> : null}
56
+ {show ? <B /> : null}
57
+ </Suspense>
58
+ );
59
+ }
60
+
61
+ const root = ReactNoop.createRoot(null);
62
+
63
+ root.render(<App show={false} />);
64
+ expect(Scheduler).toFlushAndYield([]);
65
+
66
+ root.render(<App show={true} />);
67
+ expect(Scheduler).toFlushAndYield([
68
+ 'Suspend! [A]',
69
+ 'Suspend! [B]',
70
+ 'Loading...',
71
+ ]);
72
+ expect(root).toMatchRenderedOutput(null);
73
+
74
+ Scheduler.unstable_advanceTime(2000);
75
+ expect(root).toMatchRenderedOutput(null);
76
+
77
+ scheduleCallback(NormalPriority, () => {
78
+ Scheduler.unstable_yieldValue('Resolve A');
79
+ A.resolve();
80
+ });
81
+ scheduleCallback(NormalPriority, () => {
82
+ Scheduler.unstable_yieldValue('Resolve B');
83
+ B.resolve();
84
+ });
85
+
86
+ // This resolves A and schedules a task for React to retry.
87
+ await expect(Scheduler).toFlushAndYieldThrough(['Resolve A']);
88
+
89
+ // The next task that flushes should be the one that resolves B. The render
90
+ // task should not jump the queue ahead of B.
91
+ await expect(Scheduler).toFlushAndYieldThrough(['Resolve B']);
92
+
93
+ expect(Scheduler).toFlushAndYield(['A', 'B']);
94
+ expect(root).toMatchRenderedOutput('AB');
95
+ });
96
+});
packages/shared/ReactFeatureFlags.js
+2
@@ -94,3 +94,5 @@ export const enableSuspenseCallback = false;
94
export const warnAboutDefaultPropsOnFunctionComponents = false;
95
96
export const disableLegacyContext = false;
97
+
98
+export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -41,6 +41,7 @@ export const enableUserBlockingEvents = false;
41
export const enableSuspenseCallback = false;
42
export const warnAboutDefaultPropsOnFunctionComponents = false;
43
export const disableLegacyContext = false;
44
+export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
45
46
// Only used in www builds.
47
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -36,6 +36,7 @@ export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = false;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
export const disableLegacyContext = false;
39
+export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
40
41
// Only used in www builds.
42
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.persistent.js
+1
@@ -36,6 +36,7 @@ export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = false;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
export const disableLegacyContext = false;
39
+export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
40
41
// Only used in www builds.
42
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -36,6 +36,7 @@ export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = false;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
export const disableLegacyContext = false;
39
+export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
40
41
// Only used in www builds.
42
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -36,6 +36,7 @@ export const enableUserBlockingEvents = false;
36
export const enableSuspenseCallback = true;
37
export const warnAboutDefaultPropsOnFunctionComponents = false;
38
export const disableLegacyContext = false;
39
+export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
40
41
// Only used in www builds.
42
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -22,6 +22,7 @@ export const {
22
revertPassiveEffectsChange,
23
enableUserBlockingEvents,
24
disableLegacyContext,
25
+ disableSchedulerTimeoutBasedOnReactExpirationTime,
26
} = require('ReactFeatureFlags');
27
28
// In www, we have experimental support for gathering data