Skip to content

Commit 098600c

Browse files
committed
Re-land "Fix: flushSync changes priority inside effect (#21122)"
This re-lands commit 0e3c7e1.
1 parent df420bc commit 098600c

File tree

3 files changed

+86
-22
lines changed

3 files changed

+86
-22
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

+11-11
Original file line numberDiff line numberDiff line change
@@ -1142,16 +1142,6 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
11421142

11431143
export function flushSync<A, R>(fn: A => R, a: A): R {
11441144
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-
}
11551145
executionContext |= BatchedContext;
11561146

11571147
const previousPriority = getCurrentUpdatePriority();
@@ -1168,7 +1158,17 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
11681158
// Flush the immediate callbacks that were scheduled during this batch.
11691159
// Note that this will happen even if batchedUpdates is higher up
11701160
// 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+
}
11721172
}
11731173
}
11741174

packages/react-reconciler/src/ReactFiberWorkLoop.old.js

+11-11
Original file line numberDiff line numberDiff line change
@@ -1142,16 +1142,6 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
11421142

11431143
export function flushSync<A, R>(fn: A => R, a: A): R {
11441144
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-
}
11551145
executionContext |= BatchedContext;
11561146

11571147
const previousPriority = getCurrentUpdatePriority();
@@ -1168,7 +1158,17 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
11681158
// Flush the immediate callbacks that were scheduled during this batch.
11691159
// Note that this will happen even if batchedUpdates is higher up
11701160
// 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+
}
11721172
}
11731173
}
11741174

Original file line numberDiff line numberDiff line change
@@ -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+
});

0 commit comments

Comments
 (0)