@samitouri / QOS-React-2 / commits / db3ae32b8f

flush fallbacks in tests (#16240)

In this PR, for tests (specifically, code inside an `act()` scope), we immediately trigger work that would have otherwise required a timeout. This makes it simpler to tests loading/spinner states, and makes tests resilient to changes in React. For some of our tests(specifically, ReactSuspenseWithNoopRenderer-test.internal), we _don't_ want fallbacks to immediately trigger, because we're testing intermediate states and such. Added a feature flag `flushSuspenseFallbacksInTests` to disable this behaviour on a per case basis.

Sunil Pai committed Jul 30, 2019 at 19:12 UTC db3ae32b8fe2c3fe24c4f5496aecdeab24b9d719
10 files changed +126 -9
packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
+98 -6
@@ -32,22 +32,43 @@ describe('ReactTestUtils.act()', () => {
32 concurrentRoot = ReactDOM.unstable_createRoot(dom);
33 concurrentRoot.render(el);
34 }
35 +
36 function unmountConcurrent(_dom) {
37 if (concurrentRoot !== null) {
38 concurrentRoot.unmount();
39 concurrentRoot = null;
40 }
41 }
41 - runActTests('concurrent mode', renderConcurrent, unmountConcurrent);
42 +
43 + function rerenderConcurrent(el) {
44 + concurrentRoot.render(el);
45 + }
46 +
47 + runActTests(
48 + 'concurrent mode',
49 + renderConcurrent,
50 + unmountConcurrent,
51 + rerenderConcurrent,
52 + );
53
54 // and then in sync mode
55 +
56 + let syncDom = null;
57 function renderSync(el, dom) {
58 + syncDom = dom;
59 ReactDOM.render(el, dom);
60 }
61 +
62 function unmountSync(dom) {
63 + syncDom = null;
64 ReactDOM.unmountComponentAtNode(dom);
65 }
50 - runActTests('legacy sync mode', renderSync, unmountSync);
66 +
67 + function rerenderSync(el) {
68 + ReactDOM.render(el, syncDom);
69 + }
70 +
71 + runActTests('legacy sync mode', renderSync, unmountSync, rerenderSync);
72
73 // and then in batched mode
74 let batchedRoot;
@@ -55,13 +76,19 @@ describe('ReactTestUtils.act()', () => {
76 batchedRoot = ReactDOM.unstable_createSyncRoot(dom);
77 batchedRoot.render(el);
78 }
79 +
80 function unmountBatched(dom) {
81 if (batchedRoot !== null) {
82 batchedRoot.unmount();
83 batchedRoot = null;
84 }
85 }
64 - runActTests('batched mode', renderBatched, unmountBatched);
86 +
87 + function rerenderBatched(el) {
88 + batchedRoot.render(el);
89 + }
90 +
91 + runActTests('batched mode', renderBatched, unmountBatched, rerenderBatched);
92
93 describe('unacted effects', () => {
94 function App() {
@@ -117,7 +144,7 @@ describe('ReactTestUtils.act()', () => {
144 });
145 });
146
120 -function runActTests(label, render, unmount) {
147 +function runActTests(label, render, unmount, rerender) {
148 describe(label, () => {
149 beforeEach(() => {
150 jest.resetModules();
@@ -546,7 +573,7 @@ function runActTests(label, render, unmount) {
573 expect(interactions.size).toBe(1);
574 expectedInteraction = Array.from(interactions)[0];
575
549 - render(<Component />, container);
576 + rerender(<Component />);
577 },
578 );
579 });
@@ -576,7 +603,7 @@ function runActTests(label, render, unmount) {
603 expect(interactions.size).toBe(1);
604 expectedInteraction = Array.from(interactions)[0];
605
579 - render(<Component />, secondContainer);
606 + rerender(<Component />);
607 });
608 },
609 );
@@ -693,5 +720,70 @@ function runActTests(label, render, unmount) {
720 }
721 });
722 });
723 +
724 + describe('suspense', () => {
725 + it('triggers fallbacks if available', async () => {
726 + let resolved = false;
727 + let resolve;
728 + const promise = new Promise(_resolve => {
729 + resolve = _resolve;
730 + });
731 +
732 + function Suspends() {
733 + if (resolved) {
734 + return 'was suspended';
735 + }
736 + throw promise;
737 + }
738 +
739 + function App(props) {
740 + return (
741 + <React.Suspense
742 + fallback={<span data-test-id="spinner">loading...</span>}>
743 + {props.suspend ? <Suspends /> : 'content'}
744 + </React.Suspense>
745 + );
746 + }
747 +
748 + // render something so there's content
749 + act(() => {
750 + render(<App suspend={false} />, container);
751 + });
752 +
753 + // trigger a suspendy update
754 + act(() => {
755 + rerender(<App suspend={true} />);
756 + });
757 + expect(document.querySelector('[data-test-id=spinner]')).not.toBeNull();
758 +
759 + // now render regular content again
760 + act(() => {
761 + rerender(<App suspend={false} />);
762 + });
763 + expect(document.querySelector('[data-test-id=spinner]')).toBeNull();
764 +
765 + // trigger a suspendy update with a delay
766 + React.unstable_withSuspenseConfig(
767 + () => {
768 + act(() => {
769 + rerender(<App suspend={true} />);
770 + });
771 + },
772 + {timeout: 5000},
773 + );
774 + // the spinner shows up regardless
775 + expect(document.querySelector('[data-test-id=spinner]')).not.toBeNull();
776 +
777 + // resolve the promise
778 + await act(async () => {
779 + resolved = true;
780 + resolve();
781 + });
782 +
783 + // spinner gone, content showing
784 + expect(document.querySelector('[data-test-id=spinner]')).toBeNull();
785 + expect(container.textContent).toBe('was suspended');
786 + });
787 + });
788 });
789 }
packages/react-reconciler/src/ReactFiberWorkLoop.js
+16 -3
@@ -26,6 +26,7 @@ import {
26 enableSchedulerTracing,
27 revertPassiveEffectsChange,
28 warnAboutUnmockedScheduler,
29 + flushSuspenseFallbacksInTests,
30 } from 'shared/ReactFeatureFlags';
31 import ReactSharedInternals from 'shared/ReactSharedInternals';
32 import invariant from 'shared/invariant';
@@ -993,7 +994,7 @@ function renderRoot(
994 case RootIncomplete: {
995 invariant(false, 'Should have a work-in-progress.');
996 }
996 - // Flow knows about invariant, so it compains if I add a break statement,
997 + // Flow knows about invariant, so it complains if I add a break statement,
998 // but eslint doesn't know about invariant, so it complains if I do.
999 // eslint-disable-next-line no-fallthrough
1000 case RootErrored: {
@@ -1027,7 +1028,12 @@ function renderRoot(
1028 // possible.
1029 const hasNotProcessedNewUpdates =
1030 workInProgressRootLatestProcessedExpirationTime === Sync;
1030 - if (hasNotProcessedNewUpdates && !isSync) {
1031 + if (
1032 + hasNotProcessedNewUpdates &&
1033 + !isSync &&
1034 + // do not delay if we're inside an act() scope
1035 + !(flushSuspenseFallbacksInTests && IsThisRendererActing.current)
1036 + ) {
1037 // If we have not processed any new updates during this pass, then this is
1038 // either a retry of an existing fallback state or a hidden tree.
1039 // Hidden trees shouldn't be batched with other work and after that's
@@ -1064,7 +1070,11 @@ function renderRoot(
1070 return commitRoot.bind(null, root);
1071 }
1072 case RootSuspendedWithDelay: {
1067 - if (!isSync) {
1073 + if (
1074 + !isSync &&
1075 + // do not delay if we're inside an act() scope
1076 + !(flushSuspenseFallbacksInTests && IsThisRendererActing.current)
1077 + ) {
1078 // We're suspended in a state that should be avoided. We'll try to avoid committing
1079 // it for as long as the timeouts let us.
1080 if (workInProgressRootHasPendingPing) {
@@ -1135,6 +1145,8 @@ function renderRoot(
1145 // The work completed. Ready to commit.
1146 if (
1147 !isSync &&
1148 + // do not delay if we're inside an act() scope
1149 + !(flushSuspenseFallbacksInTests && IsThisRendererActing.current) &&
1150 workInProgressRootLatestProcessedExpirationTime !== Sync &&
1151 workInProgressRootCanSuspendUsingConfig !== null
1152 ) {
@@ -2439,6 +2451,7 @@ function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
2451 }
2452 }
2453
2454 +// a 'shared' variable that changes when act() opens/closes in tests.
2455 export const IsThisRendererActing = {current: (false: boolean)};
2456
2457 export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+1
@@ -15,6 +15,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
15 ReactFeatureFlags = require('shared/ReactFeatureFlags');
16 ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
17 ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
18 + ReactFeatureFlags.flushSuspenseFallbacksInTests = false;
19 React = require('react');
20 Fragment = React.Fragment;
21 ReactNoop = require('react-noop-renderer');
packages/shared/ReactFeatureFlags.js
+4
@@ -74,6 +74,10 @@ export const warnAboutUnmockedScheduler = false;
74 // Temporary flag to revert the fix in #15650
75 export const revertPassiveEffectsChange = false;
76
77 +// For tests, we flush suspense fallbacks in an act scope;
78 +// *except* in some of our own tests, where we test incremental loading states.
79 +export const flushSuspenseFallbacksInTests = true;
80 +
81 // Changes priority of some events like mousemove to user-blocking priority,
82 // but without making them discrete. The flag exists in case it causes
83 // starvation problems.
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -36,6 +36,7 @@ export const enableFundamentalAPI = false;
36 export const enableJSXTransformAPI = false;
37 export const warnAboutUnmockedScheduler = true;
38 export const revertPassiveEffectsChange = false;
39 +export const flushSuspenseFallbacksInTests = true;
40 export const enableUserBlockingEvents = false;
41 export const enableSuspenseCallback = false;
42 export const warnAboutDefaultPropsOnFunctionComponents = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -31,6 +31,7 @@ export const enableFundamentalAPI = false;
31 export const enableJSXTransformAPI = false;
32 export const warnAboutUnmockedScheduler = false;
33 export const revertPassiveEffectsChange = false;
34 +export const flushSuspenseFallbacksInTests = true;
35 export const enableUserBlockingEvents = false;
36 export const enableSuspenseCallback = false;
37 export const warnAboutDefaultPropsOnFunctionComponents = false;
packages/shared/forks/ReactFeatureFlags.persistent.js
+1
@@ -31,6 +31,7 @@ export const enableFundamentalAPI = false;
31 export const enableJSXTransformAPI = false;
32 export const warnAboutUnmockedScheduler = true;
33 export const revertPassiveEffectsChange = false;
34 +export const flushSuspenseFallbacksInTests = true;
35 export const enableUserBlockingEvents = false;
36 export const enableSuspenseCallback = false;
37 export const warnAboutDefaultPropsOnFunctionComponents = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -31,6 +31,7 @@ export const enableFundamentalAPI = false;
31 export const enableJSXTransformAPI = false;
32 export const warnAboutUnmockedScheduler = false;
33 export const revertPassiveEffectsChange = false;
34 +export const flushSuspenseFallbacksInTests = true;
35 export const enableUserBlockingEvents = false;
36 export const enableSuspenseCallback = false;
37 export const warnAboutDefaultPropsOnFunctionComponents = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -31,6 +31,7 @@ export const enableFlareAPI = true;
31 export const enableFundamentalAPI = false;
32 export const enableJSXTransformAPI = true;
33 export const warnAboutUnmockedScheduler = true;
34 +export const flushSuspenseFallbacksInTests = true;
35 export const enableUserBlockingEvents = false;
36 export const enableSuspenseCallback = true;
37 export const warnAboutDefaultPropsOnFunctionComponents = false;
packages/shared/forks/ReactFeatureFlags.www.js
+2
@@ -80,6 +80,8 @@ export const enableSuspenseCallback = true;
80
81 export const warnAboutDefaultPropsOnFunctionComponents = false;
82
83 +export const flushSuspenseFallbacksInTests = true;
84 +
85 // Flow magic to verify the exports of this file match the original version.
86 // eslint-disable-next-line no-unused-vars
87 type Check<_X, Y: _X, X: Y = _X> = null;