Skip to content

Commit 9c9ea94

Browse files
author
Sunil Pai
authored
flush only on exiting outermost act() (#15682)
1 parent 50b50c2 commit 9c9ea94

File tree

4 files changed

+76
-33
lines changed

4 files changed

+76
-33
lines changed

‎packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,26 @@ function runActTests(label, render, unmount) {
137137
expect(container.innerHTML).toBe('5');
138138
});
139139

140+
it('should flush effects only on exiting the outermost act', () => {
141+
function App() {
142+
React.useEffect(() => {
143+
Scheduler.yieldValue(0);
144+
});
145+
return null;
146+
}
147+
// let's nest a couple of act() calls
148+
act(() => {
149+
act(() => {
150+
render(<App />, container);
151+
});
152+
// the effect wouldn't have yielded yet because
153+
// we're still inside an act() scope
154+
expect(Scheduler).toHaveYielded([]);
155+
});
156+
// but after exiting the last one, effects get flushed
157+
expect(Scheduler).toHaveYielded([0]);
158+
});
159+
140160
it('warns if a setState is called outside of act(...)', () => {
141161
let setValue = null;
142162
function App() {
@@ -281,7 +301,7 @@ function runActTests(label, render, unmount) {
281301
});
282302
});
283303
describe('asynchronous tests', () => {
284-
it('can handle timers', async () => {
304+
it('works with timeouts', async () => {
285305
function App() {
286306
let [ctr, setCtr] = React.useState(0);
287307
function doSomething() {
@@ -295,16 +315,17 @@ function runActTests(label, render, unmount) {
295315
}, []);
296316
return ctr;
297317
}
298-
act(() => {
299-
render(<App />, container);
300-
});
318+
301319
await act(async () => {
320+
render(<App />, container);
321+
// flush a little to start the timer
322+
expect(Scheduler).toFlushAndYield([]);
302323
await sleep(100);
303324
});
304325
expect(container.innerHTML).toBe('1');
305326
});
306327

307-
it('can handle async/await', async () => {
328+
it('flushes microtasks before exiting', async () => {
308329
function App() {
309330
let [ctr, setCtr] = React.useState(0);
310331
async function someAsyncFunction() {
@@ -321,10 +342,7 @@ function runActTests(label, render, unmount) {
321342
}
322343

323344
await act(async () => {
324-
act(() => {
325-
render(<App />, container);
326-
});
327-
// pending promises will close before this ends
345+
render(<App />, container);
328346
});
329347
expect(container.innerHTML).toEqual('1');
330348
});
@@ -361,7 +379,7 @@ function runActTests(label, render, unmount) {
361379
}
362380
});
363381

364-
it('commits and effects are guaranteed to be flushed', async () => {
382+
it('async commits and effects are guaranteed to be flushed', async () => {
365383
function App() {
366384
let [state, setState] = React.useState(0);
367385
async function something() {
@@ -378,17 +396,12 @@ function runActTests(label, render, unmount) {
378396
}
379397

380398
await act(async () => {
381-
act(() => {
382-
render(<App />, container);
383-
});
384-
expect(container.innerHTML).toBe('0');
385-
expect(Scheduler).toHaveYielded([0]);
399+
render(<App />, container);
386400
});
387-
// this may seem odd, but it matches user behaviour -
388-
// a flash of "0" followed by "1"
401+
// exiting act() drains effects and microtasks
389402

403+
expect(Scheduler).toHaveYielded([0, 1]);
390404
expect(container.innerHTML).toBe('1');
391-
expect(Scheduler).toHaveYielded([1]);
392405
});
393406

394407
it('propagates errors', async () => {

‎packages/react-dom/src/test-utils/ReactTestUtilsAct.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,15 @@ function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
8484
let actingUpdatesScopeDepth = 0;
8585

8686
function act(callback: () => Thenable) {
87-
let previousActingUpdatesScopeDepth;
87+
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
88+
actingUpdatesScopeDepth++;
8889
if (__DEV__) {
89-
previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
90-
actingUpdatesScopeDepth++;
9190
ReactShouldWarnActingUpdates.current = true;
9291
}
9392

9493
function onDone() {
94+
actingUpdatesScopeDepth--;
9595
if (__DEV__) {
96-
actingUpdatesScopeDepth--;
9796
if (actingUpdatesScopeDepth === 0) {
9897
ReactShouldWarnActingUpdates.current = false;
9998
}
@@ -143,6 +142,13 @@ function act(callback: () => Thenable) {
143142
called = true;
144143
result.then(
145144
() => {
145+
if (actingUpdatesScopeDepth > 1) {
146+
onDone();
147+
resolve();
148+
return;
149+
}
150+
// we're about to exit the act() scope,
151+
// now's the time to flush tasks/effects
146152
flushWorkAndMicroTasks((err: ?Error) => {
147153
onDone();
148154
if (err) {
@@ -171,7 +177,11 @@ function act(callback: () => Thenable) {
171177

172178
// flush effects until none remain, and cleanup
173179
try {
174-
flushWork();
180+
if (actingUpdatesScopeDepth === 1) {
181+
// we're about to exit the act() scope,
182+
// now's the time to flush effects
183+
flushWork();
184+
}
175185
onDone();
176186
} catch (err) {
177187
onDone();

‎packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -697,16 +697,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
697697
let actingUpdatesScopeDepth = 0;
698698

699699
function act(callback: () => Thenable) {
700-
let previousActingUpdatesScopeDepth;
700+
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
701+
actingUpdatesScopeDepth++;
701702
if (__DEV__) {
702-
previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
703-
actingUpdatesScopeDepth++;
704703
ReactShouldWarnActingUpdates.current = true;
705704
}
706705

707706
function onDone() {
707+
actingUpdatesScopeDepth--;
708708
if (__DEV__) {
709-
actingUpdatesScopeDepth--;
710709
if (actingUpdatesScopeDepth === 0) {
711710
ReactShouldWarnActingUpdates.current = false;
712711
}
@@ -756,6 +755,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
756755
called = true;
757756
result.then(
758757
() => {
758+
if (actingUpdatesScopeDepth > 1) {
759+
onDone();
760+
resolve();
761+
return;
762+
}
763+
// we're about to exit the act() scope,
764+
// now's the time to flush tasks/effects
759765
flushWorkAndMicroTasks((err: ?Error) => {
760766
onDone();
761767
if (err) {
@@ -784,7 +790,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
784790

785791
// flush effects until none remain, and cleanup
786792
try {
787-
flushWork();
793+
if (actingUpdatesScopeDepth === 1) {
794+
// we're about to exit the act() scope,
795+
// now's the time to flush effects
796+
flushWork();
797+
}
788798
onDone();
789799
} catch (err) {
790800
onDone();

‎packages/react-test-renderer/src/ReactTestRendererAct.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,15 @@ function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
6565
let actingUpdatesScopeDepth = 0;
6666

6767
function act(callback: () => Thenable) {
68-
let previousActingUpdatesScopeDepth;
68+
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
69+
actingUpdatesScopeDepth++;
6970
if (__DEV__) {
70-
previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
71-
actingUpdatesScopeDepth++;
7271
ReactShouldWarnActingUpdates.current = true;
7372
}
7473

7574
function onDone() {
75+
actingUpdatesScopeDepth--;
7676
if (__DEV__) {
77-
actingUpdatesScopeDepth--;
7877
if (actingUpdatesScopeDepth === 0) {
7978
ReactShouldWarnActingUpdates.current = false;
8079
}
@@ -124,6 +123,13 @@ function act(callback: () => Thenable) {
124123
called = true;
125124
result.then(
126125
() => {
126+
if (actingUpdatesScopeDepth > 1) {
127+
onDone();
128+
resolve();
129+
return;
130+
}
131+
// we're about to exit the act() scope,
132+
// now's the time to flush tasks/effects
127133
flushWorkAndMicroTasks((err: ?Error) => {
128134
onDone();
129135
if (err) {
@@ -152,7 +158,11 @@ function act(callback: () => Thenable) {
152158

153159
// flush effects until none remain, and cleanup
154160
try {
155-
flushWork();
161+
if (actingUpdatesScopeDepth === 1) {
162+
// we're about to exit the act() scope,
163+
// now's the time to flush effects
164+
flushWork();
165+
}
156166
onDone();
157167
} catch (err) {
158168
onDone();

0 commit comments

Comments
 (0)