Internal `act`: Flush timers at end of scope (#19788)
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.
Andrew Clark committed
Sep 8, 2020 at 23:55 UTC
e7b255341b059b4e2a109847395d0d0ba2633999
10 files changed
+48
-35
packages/react-dom/src/test-utils/ReactTestUtilsAct.js
+3
-2
@@ -126,8 +126,9 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
126
}
127
128
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();
132
enqueueTask(() => {
133
try {
134
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();
packages/react-noop-renderer/src/createReactNoop.js
+3
-2
@@ -1175,8 +1175,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
1175
}
1176
1177
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();
1181
enqueueTask(() => {
1182
try {
1183
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();
packages/react-reconciler/src/__tests__/ReactBlocks-test.js
+13
-5
@@ -14,6 +14,7 @@ let useState;
14
let Suspense;
15
let block;
16
let readString;
17
+let resolvePromises;
18
let Scheduler;
19
20
describe('ReactBlocks', () => {
@@ -28,15 +29,16 @@ describe('ReactBlocks', () => {
29
useState = React.useState;
30
Suspense = React.Suspense;
31
const cache = new Map();
32
+ let unresolved = [];
33
readString = function(text) {
34
let entry = cache.get(text);
35
if (!entry) {
36
entry = {
37
promise: new Promise(resolve => {
36
- setTimeout(() => {
38
+ unresolved.push(() => {
39
entry.resolved = true;
40
resolve();
39
- }, 100);
41
+ });
42
}),
43
resolved: false,
44
};
@@ -47,6 +49,12 @@ describe('ReactBlocks', () => {
49
}
50
return text;
51
};
52
+
53
+ resolvePromises = () => {
54
+ const res = unresolved;
55
+ unresolved = [];
56
+ res.forEach(r => r());
57
+ };
58
});
59
60
// @gate experimental
@@ -144,7 +152,7 @@ describe('ReactBlocks', () => {
152
expect(ReactNoop).toMatchRenderedOutput('Loading...');
153
154
await ReactNoop.act(async () => {
147
- jest.advanceTimersByTime(1000);
155
+ resolvePromises();
156
});
157
158
expect(ReactNoop).toMatchRenderedOutput(<span>Name: Sebastian</span>);
@@ -291,7 +299,7 @@ describe('ReactBlocks', () => {
299
ReactNoop.render(<App Page={loadParent('Sebastian')} />);
300
});
301
await ReactNoop.act(async () => {
294
- jest.advanceTimersByTime(1000);
302
+ resolvePromises();
303
});
304
expect(ReactNoop).toMatchRenderedOutput(<span>Name: Sebastian</span>);
305
});
@@ -336,7 +344,7 @@ describe('ReactBlocks', () => {
344
});
345
await ReactNoop.act(async () => {
346
_setSuspend(false);
339
- jest.advanceTimersByTime(1000);
347
+ resolvePromises();
348
});
349
expect(ReactNoop).toMatchRenderedOutput(<span>Sebastian</span>);
350
});
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+15
-24
@@ -575,7 +575,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
575
<Suspense fallback="Loading...">
576
<Text text="Sibling" />
577
{shouldSuspend ? (
578
- <AsyncText ms={10000} text={'Step ' + step} />
578
+ <AsyncText text={'Step ' + step} />
579
) : (
580
<Text text={'Step ' + step} />
581
)}
@@ -2595,7 +2595,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
2595
}
2596
return (
2597
<Suspense fallback={<Text text="Loading..." />}>
2598
- <AsyncText text={page} ms={5000} />
2598
+ <AsyncText text={page} />
2599
</Suspense>
2600
);
2601
}
@@ -2616,8 +2616,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
2616
});
2617
2618
// Later we load the data.
2619
- Scheduler.unstable_advanceTime(5000);
2620
- await advanceTimers(5000);
2619
+ await resolveText('A');
2620
expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
2621
expect(Scheduler).toFlushAndYield(['A']);
2622
expect(ReactNoop.getChildren()).toEqual([span('A')]);
@@ -2635,8 +2634,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
2634
});
2635
2636
// Later we load the data.
2638
- Scheduler.unstable_advanceTime(3000);
2639
- await advanceTimers(3000);
2637
+ await resolveText('B');
2638
expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
2639
expect(Scheduler).toFlushAndYield(['B']);
2640
expect(ReactNoop.getChildren()).toEqual([span('B')]);
@@ -2754,12 +2752,14 @@ describe('ReactSuspenseWithNoopRenderer', () => {
2752
);
2753
});
2754
2755
+ // TODO: This test is specifically about avoided commits that suspend for a
2756
+ // JND. We may remove this behavior.
2757
it("suspended commit remains suspended even if there's another update at same expiration", async () => {
2758
// Regression test
2759
function App({text}) {
2760
return (
2761
<Suspense fallback="Loading...">
2762
- <AsyncText ms={2000} text={text} />
2762
+ <AsyncText text={text} />
2763
</Suspense>
2764
);
2765
}
@@ -2768,34 +2768,28 @@ describe('ReactSuspenseWithNoopRenderer', () => {
2768
await ReactNoop.act(async () => {
2769
root.render(<App text="Initial" />);
2770
});
2771
+ expect(Scheduler).toHaveYielded(['Suspend! [Initial]']);
2772
2773
// Resolve initial render
2774
await ReactNoop.act(async () => {
2774
- Scheduler.unstable_advanceTime(2000);
2775
- await advanceTimers(2000);
2775
+ await resolveText('Initial');
2776
});
2777
- expect(Scheduler).toHaveYielded([
2778
- 'Suspend! [Initial]',
2779
- 'Promise resolved [Initial]',
2780
- 'Initial',
2781
- ]);
2777
+ expect(Scheduler).toHaveYielded(['Promise resolved [Initial]', 'Initial']);
2778
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
2779
2784
- // Update. Since showing a fallback would hide content that's already
2785
- // visible, it should suspend for a bit without committing.
2780
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.
2783
root.render(<App text="First update" />);
2788
-
2784
expect(Scheduler).toFlushAndYield(['Suspend! [First update]']);
2785
+
2786
// Should not display a fallback
2787
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
2792
- });
2788
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.
2790
root.render(<App text="Second update" />);
2797
-
2791
expect(Scheduler).toFlushAndYield(['Suspend! [Second update]']);
2792
+
2793
// Should not display a fallback
2794
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
2795
});
@@ -3989,9 +3983,6 @@ describe('ReactSuspenseWithNoopRenderer', () => {
3983
await ReactNoop.act(async () => {
3984
root.render(<App show={true} />);
3985
});
3992
- // TODO: `act` should have already flushed the placeholder, so this
3993
- // runAllTimers call should be unnecessary.
3994
- jest.runAllTimers();
3986
expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']);
3987
expect(root).toMatchRenderedOutput(
3988
<>
packages/react-test-renderer/src/ReactTestRenderer.js
+3
-2
@@ -684,8 +684,9 @@ function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
684
}
685
686
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();
690
enqueueTask(() => {
691
try {
692
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();
scripts/rollup/validate/eslintrc.cjs.js
+1
@@ -42,6 +42,7 @@ module.exports = {
42
43
// jest
44
expect: true,
45
+ jest: true,
46
},
47
parserOptions: {
48
ecmaVersion: 5,
scripts/rollup/validate/eslintrc.cjs2015.js
+1
@@ -42,6 +42,7 @@ module.exports = {
42
43
// jest
44
expect: true,
45
+ jest: true,
46
},
47
parserOptions: {
48
ecmaVersion: 2015,
scripts/rollup/validate/eslintrc.fb.js
+3
@@ -36,6 +36,9 @@ module.exports = {
36
// Flight
37
Uint8Array: true,
38
Promise: true,
39
+
40
+ // jest
41
+ jest: true,
42
},
43
parserOptions: {
44
ecmaVersion: 5,
scripts/rollup/validate/eslintrc.rn.js
+3
@@ -31,6 +31,9 @@ module.exports = {
31
ArrayBuffer: true,
32
33
TaskController: true,
34
+
35
+ // jest
36
+ jest: true,
37
},
38
parserOptions: {
39
ecmaVersion: 5,
scripts/rollup/validate/eslintrc.umd.js
+3
@@ -43,6 +43,9 @@ module.exports = {
43
// Flight Webpack
44
__webpack_chunk_load__: true,
45
__webpack_require__: true,
46
+
47
+ // jest
48
+ jest: true,
49
},
50
parserOptions: {
51
ecmaVersion: 5,