Skip to content

Commit

Permalink
flush only on exiting outermost act() (#15682)
Browse files Browse the repository at this point in the history
  • Loading branch information
Sunil Pai committed May 21, 2019
1 parent 50b50c2 commit 9c9ea94
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 33 deletions.
49 changes: 31 additions & 18 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
View file
Expand Up @@ -137,6 +137,26 @@ function runActTests(label, render, unmount) {
expect(container.innerHTML).toBe('5');
});

it('should flush effects only on exiting the outermost act', () => {
function App() {
React.useEffect(() => {
Scheduler.yieldValue(0);
});
return null;
}
// let's nest a couple of act() calls
act(() => {
act(() => {
render(<App />, container);
});
// the effect wouldn't have yielded yet because
// we're still inside an act() scope
expect(Scheduler).toHaveYielded([]);
});
// but after exiting the last one, effects get flushed
expect(Scheduler).toHaveYielded([0]);
});

it('warns if a setState is called outside of act(...)', () => {
let setValue = null;
function App() {
Expand Down Expand Up @@ -281,7 +301,7 @@ function runActTests(label, render, unmount) {
});
});
describe('asynchronous tests', () => {
it('can handle timers', async () => {
it('works with timeouts', async () => {
function App() {
let [ctr, setCtr] = React.useState(0);
function doSomething() {
Expand All @@ -295,16 +315,17 @@ function runActTests(label, render, unmount) {
}, []);
return ctr;
}
act(() => {
render(<App />, container);
});

await act(async () => {
render(<App />, container);
// flush a little to start the timer
expect(Scheduler).toFlushAndYield([]);
await sleep(100);
});
expect(container.innerHTML).toBe('1');
});

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

await act(async () => {
act(() => {
render(<App />, container);
});
// pending promises will close before this ends
render(<App />, container);
});
expect(container.innerHTML).toEqual('1');
});
Expand Down Expand Up @@ -361,7 +379,7 @@ function runActTests(label, render, unmount) {
}
});

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

await act(async () => {
act(() => {
render(<App />, container);
});
expect(container.innerHTML).toBe('0');
expect(Scheduler).toHaveYielded([0]);
render(<App />, container);
});
// this may seem odd, but it matches user behaviour -
// a flash of "0" followed by "1"
// exiting act() drains effects and microtasks

expect(Scheduler).toHaveYielded([0, 1]);
expect(container.innerHTML).toBe('1');
expect(Scheduler).toHaveYielded([1]);
});

it('propagates errors', async () => {
Expand Down
20 changes: 15 additions & 5 deletions packages/react-dom/src/test-utils/ReactTestUtilsAct.js
View file
Expand Up @@ -84,16 +84,15 @@ function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth;
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
ReactShouldWarnActingUpdates.current = true;
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
actingUpdatesScopeDepth--;
if (actingUpdatesScopeDepth === 0) {
ReactShouldWarnActingUpdates.current = false;
}
Expand Down Expand Up @@ -143,6 +142,13 @@ function act(callback: () => Thenable) {
called = true;
result.then(
() => {
if (actingUpdatesScopeDepth > 1) {
onDone();
resolve();
return;
}
// we're about to exit the act() scope,
// now's the time to flush tasks/effects
flushWorkAndMicroTasks((err: ?Error) => {
onDone();
if (err) {
Expand Down Expand Up @@ -171,7 +177,11 @@ function act(callback: () => Thenable) {

// flush effects until none remain, and cleanup
try {
flushWork();
if (actingUpdatesScopeDepth === 1) {
// we're about to exit the act() scope,
// now's the time to flush effects
flushWork();
}
onDone();
} catch (err) {
onDone();
Expand Down
20 changes: 15 additions & 5 deletions packages/react-noop-renderer/src/createReactNoop.js
View file
Expand Up @@ -697,16 +697,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth;
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
ReactShouldWarnActingUpdates.current = true;
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
actingUpdatesScopeDepth--;
if (actingUpdatesScopeDepth === 0) {
ReactShouldWarnActingUpdates.current = false;
}
Expand Down Expand Up @@ -756,6 +755,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
called = true;
result.then(
() => {
if (actingUpdatesScopeDepth > 1) {
onDone();
resolve();
return;
}
// we're about to exit the act() scope,
// now's the time to flush tasks/effects
flushWorkAndMicroTasks((err: ?Error) => {
onDone();
if (err) {
Expand Down Expand Up @@ -784,7 +790,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

// flush effects until none remain, and cleanup
try {
flushWork();
if (actingUpdatesScopeDepth === 1) {
// we're about to exit the act() scope,
// now's the time to flush effects
flushWork();
}
onDone();
} catch (err) {
onDone();
Expand Down
20 changes: 15 additions & 5 deletions packages/react-test-renderer/src/ReactTestRendererAct.js
View file
Expand Up @@ -65,16 +65,15 @@ function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth;
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;
ReactShouldWarnActingUpdates.current = true;
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
actingUpdatesScopeDepth--;
if (actingUpdatesScopeDepth === 0) {
ReactShouldWarnActingUpdates.current = false;
}
Expand Down Expand Up @@ -124,6 +123,13 @@ function act(callback: () => Thenable) {
called = true;
result.then(
() => {
if (actingUpdatesScopeDepth > 1) {
onDone();
resolve();
return;
}
// we're about to exit the act() scope,
// now's the time to flush tasks/effects
flushWorkAndMicroTasks((err: ?Error) => {
onDone();
if (err) {
Expand Down Expand Up @@ -152,7 +158,11 @@ function act(callback: () => Thenable) {

// flush effects until none remain, and cleanup
try {
flushWork();
if (actingUpdatesScopeDepth === 1) {
// we're about to exit the act() scope,
// now's the time to flush effects
flushWork();
}
onDone();
} catch (err) {
onDone();
Expand Down

0 comments on commit 9c9ea94

Please sign in to comment.