Skip to content

Commit 574e57c

Browse files
committed
Internal act: Flush timers at end of scope
If there are any suspended fallbacks at the end of the `act` scope, force them to display by running the pending timers (i.e. `setTimeout`). The public implementation of `act` achieves the same behavior with an extra check in the work loop (`shouldForceFlushFallbacks`). Since our internal `act` needs to work in both development and production, without additional runtime checks, we instead rely on Jest's mock timers. This doesn't not affect refresh transitions, which are meant to delay indefinitely, because in that case we exit the work loop without posting a timer.
1 parent d17086c commit 574e57c

File tree

8 files changed

+42
-35
lines changed

8 files changed

+42
-35
lines changed

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,9 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
126126
}
127127

128128
function flushActWork(resolve, reject) {
129-
// TODO: Run timers to flush suspended fallbacks
130-
// jest.runOnlyPendingTimers();
129+
// Flush suspended fallbacks
130+
// $FlowFixMe: Flow doesn't know about global Jest object
131+
jest.runOnlyPendingTimers();
131132
enqueueTask(() => {
132133
try {
133134
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -1175,8 +1175,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
11751175
}
11761176

11771177
function flushActWork(resolve, reject) {
1178-
// TODO: Run timers to flush suspended fallbacks
1179-
// jest.runOnlyPendingTimers();
1178+
// Flush suspended fallbacks
1179+
// $FlowFixMe: Flow doesn't know about global Jest object
1180+
jest.runOnlyPendingTimers();
11801181
enqueueTask(() => {
11811182
try {
11821183
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();

packages/react-reconciler/src/__tests__/ReactBlocks-test.js

+13-5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ let useState;
1414
let Suspense;
1515
let block;
1616
let readString;
17+
let resolvePromises;
1718
let Scheduler;
1819

1920
describe('ReactBlocks', () => {
@@ -28,15 +29,16 @@ describe('ReactBlocks', () => {
2829
useState = React.useState;
2930
Suspense = React.Suspense;
3031
const cache = new Map();
32+
let unresolved = [];
3133
readString = function(text) {
3234
let entry = cache.get(text);
3335
if (!entry) {
3436
entry = {
3537
promise: new Promise(resolve => {
36-
setTimeout(() => {
38+
unresolved.push(() => {
3739
entry.resolved = true;
3840
resolve();
39-
}, 100);
41+
});
4042
}),
4143
resolved: false,
4244
};
@@ -47,6 +49,12 @@ describe('ReactBlocks', () => {
4749
}
4850
return text;
4951
};
52+
53+
resolvePromises = () => {
54+
const res = unresolved;
55+
unresolved = [];
56+
res.forEach(r => r());
57+
};
5058
});
5159

5260
// @gate experimental
@@ -144,7 +152,7 @@ describe('ReactBlocks', () => {
144152
expect(ReactNoop).toMatchRenderedOutput('Loading...');
145153

146154
await ReactNoop.act(async () => {
147-
jest.advanceTimersByTime(1000);
155+
resolvePromises();
148156
});
149157

150158
expect(ReactNoop).toMatchRenderedOutput(<span>Name: Sebastian</span>);
@@ -291,7 +299,7 @@ describe('ReactBlocks', () => {
291299
ReactNoop.render(<App Page={loadParent('Sebastian')} />);
292300
});
293301
await ReactNoop.act(async () => {
294-
jest.advanceTimersByTime(1000);
302+
resolvePromises();
295303
});
296304
expect(ReactNoop).toMatchRenderedOutput(<span>Name: Sebastian</span>);
297305
});
@@ -336,7 +344,7 @@ describe('ReactBlocks', () => {
336344
});
337345
await ReactNoop.act(async () => {
338346
_setSuspend(false);
339-
jest.advanceTimersByTime(1000);
347+
resolvePromises();
340348
});
341349
expect(ReactNoop).toMatchRenderedOutput(<span>Sebastian</span>);
342350
});

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js

+15-24
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
575575
<Suspense fallback="Loading...">
576576
<Text text="Sibling" />
577577
{shouldSuspend ? (
578-
<AsyncText ms={10000} text={'Step ' + step} />
578+
<AsyncText text={'Step ' + step} />
579579
) : (
580580
<Text text={'Step ' + step} />
581581
)}
@@ -2595,7 +2595,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
25952595
}
25962596
return (
25972597
<Suspense fallback={<Text text="Loading..." />}>
2598-
<AsyncText text={page} ms={5000} />
2598+
<AsyncText text={page} />
25992599
</Suspense>
26002600
);
26012601
}
@@ -2616,8 +2616,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
26162616
});
26172617

26182618
// Later we load the data.
2619-
Scheduler.unstable_advanceTime(5000);
2620-
await advanceTimers(5000);
2619+
await resolveText('A');
26212620
expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
26222621
expect(Scheduler).toFlushAndYield(['A']);
26232622
expect(ReactNoop.getChildren()).toEqual([span('A')]);
@@ -2635,8 +2634,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
26352634
});
26362635

26372636
// Later we load the data.
2638-
Scheduler.unstable_advanceTime(3000);
2639-
await advanceTimers(3000);
2637+
await resolveText('B');
26402638
expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
26412639
expect(Scheduler).toFlushAndYield(['B']);
26422640
expect(ReactNoop.getChildren()).toEqual([span('B')]);
@@ -2754,12 +2752,14 @@ describe('ReactSuspenseWithNoopRenderer', () => {
27542752
);
27552753
});
27562754

2755+
// TODO: This test is specifically about avoided commits that suspend for a
2756+
// JND. We may remove this behavior.
27572757
it("suspended commit remains suspended even if there's another update at same expiration", async () => {
27582758
// Regression test
27592759
function App({text}) {
27602760
return (
27612761
<Suspense fallback="Loading...">
2762-
<AsyncText ms={2000} text={text} />
2762+
<AsyncText text={text} />
27632763
</Suspense>
27642764
);
27652765
}
@@ -2768,34 +2768,28 @@ describe('ReactSuspenseWithNoopRenderer', () => {
27682768
await ReactNoop.act(async () => {
27692769
root.render(<App text="Initial" />);
27702770
});
2771+
expect(Scheduler).toHaveYielded(['Suspend! [Initial]']);
27712772

27722773
// Resolve initial render
27732774
await ReactNoop.act(async () => {
2774-
Scheduler.unstable_advanceTime(2000);
2775-
await advanceTimers(2000);
2775+
await resolveText('Initial');
27762776
});
2777-
expect(Scheduler).toHaveYielded([
2778-
'Suspend! [Initial]',
2779-
'Promise resolved [Initial]',
2780-
'Initial',
2781-
]);
2777+
expect(Scheduler).toHaveYielded(['Promise resolved [Initial]', 'Initial']);
27822778
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
27832779

2784-
// Update. Since showing a fallback would hide content that's already
2785-
// visible, it should suspend for a bit without committing.
27862780
await ReactNoop.act(async () => {
2781+
// Update. Since showing a fallback would hide content that's already
2782+
// visible, it should suspend for a JND without committing.
27872783
root.render(<App text="First update" />);
2788-
27892784
expect(Scheduler).toFlushAndYield(['Suspend! [First update]']);
2785+
27902786
// Should not display a fallback
27912787
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
2792-
});
27932788

2794-
// Update again. This should also suspend for a bit.
2795-
await ReactNoop.act(async () => {
2789+
// Update again. This should also suspend for a JND.
27962790
root.render(<App text="Second update" />);
2797-
27982791
expect(Scheduler).toFlushAndYield(['Suspend! [Second update]']);
2792+
27992793
// Should not display a fallback
28002794
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
28012795
});
@@ -3989,9 +3983,6 @@ describe('ReactSuspenseWithNoopRenderer', () => {
39893983
await ReactNoop.act(async () => {
39903984
root.render(<App show={true} />);
39913985
});
3992-
// TODO: `act` should have already flushed the placeholder, so this
3993-
// runAllTimers call should be unnecessary.
3994-
jest.runAllTimers();
39953986
expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']);
39963987
expect(root).toMatchRenderedOutput(
39973988
<>

packages/react-test-renderer/src/ReactTestRenderer.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -684,8 +684,9 @@ function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
684684
}
685685

686686
function flushActWork(resolve, reject) {
687-
// TODO: Run timers to flush suspended fallbacks
688-
// jest.runOnlyPendingTimers();
687+
// Flush suspended fallbacks
688+
// $FlowFixMe: Flow doesn't know about global Jest object
689+
jest.runOnlyPendingTimers();
689690
enqueueTask(() => {
690691
try {
691692
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();

scripts/rollup/validate/eslintrc.cjs.js

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ module.exports = {
4242

4343
// jest
4444
expect: true,
45+
jest: true,
4546
},
4647
parserOptions: {
4748
ecmaVersion: 5,

scripts/rollup/validate/eslintrc.cjs2015.js

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ module.exports = {
4242

4343
// jest
4444
expect: true,
45+
jest: true,
4546
},
4647
parserOptions: {
4748
ecmaVersion: 2015,

scripts/rollup/validate/eslintrc.umd.js

+3
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ module.exports = {
4343
// Flight Webpack
4444
__webpack_chunk_load__: true,
4545
__webpack_require__: true,
46+
47+
// jest
48+
jest: true,
4649
},
4750
parserOptions: {
4851
ecmaVersion: 5,

0 commit comments

Comments
 (0)