Re-land "Fix: flushSync changes priority inside effect (#21122)"
This re-lands commit 0e3c7e1d62efb6238b69e5295d45b9bd2dcf9181.
Andrew Clark committed
May 3, 2021 at 13:34 UTC
098600c42addca272bcb833bf55f53a65126e8aa
3 files changed
+86
-22
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+11
-11
@@ -1142,16 +1142,6 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
1142
1143
export function flushSync<A, R>(fn: A => R, a: A): R {
1144
const prevExecutionContext = executionContext;
1145
- if ((prevExecutionContext & (RenderContext | CommitContext)) !== NoContext) {
1146
- if (__DEV__) {
1147
- console.error(
1148
- 'flushSync was called from inside a lifecycle method. React cannot ' +
1149
- 'flush when React is already rendering. Consider moving this call to ' +
1150
- 'a scheduler task or micro task.',
1151
- );
1152
- }
1153
- return fn(a);
1154
- }
1145
executionContext |= BatchedContext;
1146
1147
const previousPriority = getCurrentUpdatePriority();
@@ -1168,7 +1158,17 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
1158
// Flush the immediate callbacks that were scheduled during this batch.
1159
// Note that this will happen even if batchedUpdates is higher up
1160
// the stack.
1171
- flushSyncCallbacks();
1161
+ if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1162
+ flushSyncCallbacks();
1163
+ } else {
1164
+ if (__DEV__) {
1165
+ console.error(
1166
+ 'flushSync was called from inside a lifecycle method. React cannot ' +
1167
+ 'flush when React is already rendering. Consider moving this call to ' +
1168
+ 'a scheduler task or micro task.',
1169
+ );
1170
+ }
1171
+ }
1172
}
1173
}
1174
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+11
-11
@@ -1142,16 +1142,6 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
1142
1143
export function flushSync<A, R>(fn: A => R, a: A): R {
1144
const prevExecutionContext = executionContext;
1145
- if ((prevExecutionContext & (RenderContext | CommitContext)) !== NoContext) {
1146
- if (__DEV__) {
1147
- console.error(
1148
- 'flushSync was called from inside a lifecycle method. React cannot ' +
1149
- 'flush when React is already rendering. Consider moving this call to ' +
1150
- 'a scheduler task or micro task.',
1151
- );
1152
- }
1153
- return fn(a);
1154
- }
1145
executionContext |= BatchedContext;
1146
1147
const previousPriority = getCurrentUpdatePriority();
@@ -1168,7 +1158,17 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
1158
// Flush the immediate callbacks that were scheduled during this batch.
1159
// Note that this will happen even if batchedUpdates is higher up
1160
// the stack.
1171
- flushSyncCallbacks();
1161
+ if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1162
+ flushSyncCallbacks();
1163
+ } else {
1164
+ if (__DEV__) {
1165
+ console.error(
1166
+ 'flushSync was called from inside a lifecycle method. React cannot ' +
1167
+ 'flush when React is already rendering. Consider moving this call to ' +
1168
+ 'a scheduler task or micro task.',
1169
+ );
1170
+ }
1171
+ }
1172
}
1173
}
1174
packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
new
+64
@@ -0,0 +1,64 @@
1
+let React;
2
+let ReactNoop;
3
+let Scheduler;
4
+let useState;
5
+let useEffect;
6
+
7
+describe('ReactFlushSync', () => {
8
+ beforeEach(() => {
9
+ jest.resetModules();
10
+
11
+ React = require('react');
12
+ ReactNoop = require('react-noop-renderer');
13
+ Scheduler = require('scheduler');
14
+ useState = React.useState;
15
+ useEffect = React.useEffect;
16
+ });
17
+
18
+ function Text({text}) {
19
+ Scheduler.unstable_yieldValue(text);
20
+ return text;
21
+ }
22
+
23
+ // @gate experimental || !enableSyncDefaultUpdates
24
+ test('changes priority of updates in useEffect', async () => {
25
+ function App() {
26
+ const [syncState, setSyncState] = useState(0);
27
+ const [state, setState] = useState(0);
28
+ useEffect(() => {
29
+ if (syncState !== 1) {
30
+ setState(1);
31
+ ReactNoop.flushSync(() => setSyncState(1));
32
+ }
33
+ }, [syncState, state]);
34
+ return <Text text={`${syncState}, ${state}`} />;
35
+ }
36
+
37
+ const root = ReactNoop.createRoot();
38
+ await ReactNoop.act(async () => {
39
+ if (gate(flags => flags.enableSyncDefaultUpdates)) {
40
+ React.unstable_startTransition(() => {
41
+ root.render(<App />);
42
+ });
43
+ } else {
44
+ root.render(<App />);
45
+ }
46
+ // This will yield right before the passive effect fires
47
+ expect(Scheduler).toFlushUntilNextPaint(['0, 0']);
48
+
49
+ // The passive effect will schedule a sync update and a normal update.
50
+ // They should commit in two separate batches. First the sync one.
51
+ expect(() => {
52
+ expect(Scheduler).toFlushUntilNextPaint(['1, 0']);
53
+ }).toErrorDev('flushSync was called from inside a lifecycle method');
54
+
55
+ // The remaining update is not sync
56
+ ReactNoop.flushSync();
57
+ expect(Scheduler).toHaveYielded([]);
58
+
59
+ // Now flush it.
60
+ expect(Scheduler).toFlushUntilNextPaint(['1, 1']);
61
+ });
62
+ expect(root).toMatchRenderedOutput('1, 1');
63
+ });
64
+});