@samitouri / QOS-React / commits / b93f3e7d2d

Fix `act` bundle size regression (#19832)

Adds back the `TestUtils.act` implementation that I had removed in #19745. This version of `act` is implemented in "userspace" (i.e. not the reconciler), so it doesn't add to the production bundle size. I had removed this in #19745 in favor of the `act` exported by the reconciler because I thought we would remove support for `act` in production in the impending major release. (It currently warns.) However, we've since decided to continue supporting `act` in prod for now, so that it doesn't block people from upgrading to v17. We'll drop support in a future major release. So, to avoid bloating the production bundle size, we need to move the public version of `act` back to "userspace", like it was before. This doesn't negate the main goal of #19745, though, which was to decouple the public version(s) of `act` from the internal one that we use to test React itself.

Andrew Clark committed Sep 14, 2020 at 12:11 UTC b93f3e7d2d595444c6ee2964d4cce2e17f4ffa28
5 files changed +234 -6
packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
+13
@@ -728,7 +728,20 @@ function runActTests(label, render, unmount, rerender) {
728 describe('suspense', () => {
729 if (__DEV__ && __EXPERIMENTAL__) {
730 // todo - remove __DEV__ check once we start using testing builds
731 +
732 it('triggers fallbacks if available', async () => {
733 + if (label !== 'legacy mode') {
734 + // FIXME: Support for Blocking* and Concurrent Mode were
735 + // intentionally removed from the public version of `act`. It will
736 + // be added back in a future major version, before Blocking and and
737 + // Concurrent Mode are officially released. Consider disabling all
738 + // non-Legacy tests in this suite until then.
739 + //
740 + // *Blocking Mode actually does happen to work, though
741 + // not "officially" since it's an unreleased feature.
742 + return;
743 + }
744 +
745 let resolved = false;
746 let resolve;
747 const promise = new Promise(_resolve => {
packages/react-dom/src/client/ReactDOM.js
+1 -3
@@ -37,7 +37,6 @@ import {
37 attemptHydrationAtCurrentPriority,
38 runWithPriority,
39 getCurrentUpdateLanePriority,
40 - act,
40 } from 'react-reconciler/src/ReactFiberReconciler';
41 import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
42 import {canUseDOM} from 'shared/ExecutionEnvironment';
@@ -184,9 +183,8 @@ const Internals = {
183 enqueueStateRestore,
184 restoreStateIfNeeded,
185 flushPassiveEffects,
187 - // TODO: These are related to `act`, not events. Move to separate key?
186 + // TODO: This is related to `act`, not events. Move to separate key?
187 IsThisRendererActing,
189 - act,
188 ],
189 };
190
packages/react-dom/src/test-utils/ReactTestUtils.js
+3 -3
@@ -18,7 +18,8 @@ import {
18 import {SyntheticEvent} from '../events/SyntheticEvent';
19 import invariant from 'shared/invariant';
20 import {ELEMENT_NODE} from '../shared/HTMLNodeType';
21 -import {unstable_concurrentAct} from './ReactTestUtilsAct';
21 +import {act} from './ReactTestUtilsPublicAct';
22 +import {unstable_concurrentAct} from './ReactTestUtilsInternalAct';
23 import {
24 rethrowCaughtError,
25 invokeGuardedCallbackAndCatchFirstError,
@@ -33,9 +34,8 @@ const getFiberCurrentPropsFromNode = EventInternals[2];
34 const enqueueStateRestore = EventInternals[3];
35 const restoreStateIfNeeded = EventInternals[4];
36 // const flushPassiveEffects = EventInternals[5];
36 -// TODO: These are related to `act`, not events. Move to separate key?
37 +// TODO: This is related to `act`, not events. Move to separate key?
38 // const IsThisRendererActing = EventInternals[6];
38 -const act = EventInternals[7];
39
40 function Event(suffix) {}
41
packages/react-dom/src/test-utils/ReactTestUtilsInternalAct.js renamed
packages/react-dom/src/test-utils/ReactTestUtilsPublicAct.js new
+217
@@ -0,0 +1,217 @@
1 +/**
2 + * Copyright (c) Facebook, Inc. and its affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @flow
8 + */
9 +
10 +import type {Thenable} from 'shared/ReactTypes';
11 +
12 +import * as ReactDOM from 'react-dom';
13 +import ReactSharedInternals from 'shared/ReactSharedInternals';
14 +import enqueueTask from 'shared/enqueueTask';
15 +import * as Scheduler from 'scheduler';
16 +
17 +// Keep in sync with ReactDOM.js, and ReactTestUtils.js:
18 +const EventInternals =
19 + ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events;
20 +// const getInstanceFromNode = EventInternals[0];
21 +// const getNodeFromInstance = EventInternals[1];
22 +// const getFiberCurrentPropsFromNode = EventInternals[2];
23 +// const enqueueStateRestore = EventInternals[3];
24 +// const restoreStateIfNeeded = EventInternals[4];
25 +const flushPassiveEffects = EventInternals[5];
26 +const IsThisRendererActing = EventInternals[6];
27 +
28 +const batchedUpdates = ReactDOM.unstable_batchedUpdates;
29 +
30 +const {IsSomeRendererActing} = ReactSharedInternals;
31 +
32 +// This is the public version of `ReactTestUtils.act`. It is implemented in
33 +// "userspace" (i.e. not the reconciler), so that it doesn't add to the
34 +// production bundle size.
35 +// TODO: Remove this implementation of `act` in favor of the one exported by
36 +// the reconciler. To do this, we must first drop support for `act` in
37 +// production mode.
38 +
39 +// TODO: Remove support for the mock scheduler build, which was only added for
40 +// the purposes of internal testing. Internal tests should use
41 +// `unstable_concurrentAct` instead.
42 +const isSchedulerMocked =
43 + typeof Scheduler.unstable_flushAllWithoutAsserting === 'function';
44 +const flushWork =
45 + Scheduler.unstable_flushAllWithoutAsserting ||
46 + function() {
47 + let didFlushWork = false;
48 + while (flushPassiveEffects()) {
49 + didFlushWork = true;
50 + }
51 +
52 + return didFlushWork;
53 + };
54 +
55 +function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
56 + try {
57 + flushWork();
58 + enqueueTask(() => {
59 + if (flushWork()) {
60 + flushWorkAndMicroTasks(onDone);
61 + } else {
62 + onDone();
63 + }
64 + });
65 + } catch (err) {
66 + onDone(err);
67 + }
68 +}
69 +
70 +// we track the 'depth' of the act() calls with this counter,
71 +// so we can tell if any async act() calls try to run in parallel.
72 +
73 +let actingUpdatesScopeDepth = 0;
74 +let didWarnAboutUsingActInProd = false;
75 +
76 +export function act(callback: () => Thenable<mixed>): Thenable<void> {
77 + if (!__DEV__) {
78 + if (didWarnAboutUsingActInProd === false) {
79 + didWarnAboutUsingActInProd = true;
80 + // eslint-disable-next-line react-internal/no-production-logging
81 + console.error(
82 + 'act(...) is not supported in production builds of React, and might not behave as expected.',
83 + );
84 + }
85 + }
86 + const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
87 + actingUpdatesScopeDepth++;
88 +
89 + const previousIsSomeRendererActing = IsSomeRendererActing.current;
90 + const previousIsThisRendererActing = IsThisRendererActing.current;
91 + IsSomeRendererActing.current = true;
92 + IsThisRendererActing.current = true;
93 +
94 + function onDone() {
95 + actingUpdatesScopeDepth--;
96 + IsSomeRendererActing.current = previousIsSomeRendererActing;
97 + IsThisRendererActing.current = previousIsThisRendererActing;
98 + if (__DEV__) {
99 + if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
100 + // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
101 + console.error(
102 + 'You seem to have overlapping act() calls, this is not supported. ' +
103 + 'Be sure to await previous act() calls before making a new one. ',
104 + );
105 + }
106 + }
107 + }
108 +
109 + let result;
110 + try {
111 + result = batchedUpdates(callback);
112 + } catch (error) {
113 + // on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth
114 + onDone();
115 + throw error;
116 + }
117 +
118 + if (
119 + result !== null &&
120 + typeof result === 'object' &&
121 + typeof result.then === 'function'
122 + ) {
123 + // setup a boolean that gets set to true only
124 + // once this act() call is await-ed
125 + let called = false;
126 + if (__DEV__) {
127 + if (typeof Promise !== 'undefined') {
128 + //eslint-disable-next-line no-undef
129 + Promise.resolve()
130 + .then(() => {})
131 + .then(() => {
132 + if (called === false) {
133 + console.error(
134 + 'You called act(async () => ...) without await. ' +
135 + 'This could lead to unexpected testing behaviour, interleaving multiple act ' +
136 + 'calls and mixing their scopes. You should - await act(async () => ...);',
137 + );
138 + }
139 + });
140 + }
141 + }
142 +
143 + // in the async case, the returned thenable runs the callback, flushes
144 + // effects and microtasks in a loop until flushPassiveEffects() === false,
145 + // and cleans up
146 + return {
147 + then(resolve, reject) {
148 + called = true;
149 + result.then(
150 + () => {
151 + if (
152 + actingUpdatesScopeDepth > 1 ||
153 + (isSchedulerMocked === true &&
154 + previousIsSomeRendererActing === true)
155 + ) {
156 + onDone();
157 + resolve();
158 + return;
159 + }
160 + // we're about to exit the act() scope,
161 + // now's the time to flush tasks/effects
162 + flushWorkAndMicroTasks((err: ?Error) => {
163 + onDone();
164 + if (err) {
165 + reject(err);
166 + } else {
167 + resolve();
168 + }
169 + });
170 + },
171 + err => {
172 + onDone();
173 + reject(err);
174 + },
175 + );
176 + },
177 + };
178 + } else {
179 + if (__DEV__) {
180 + if (result !== undefined) {
181 + console.error(
182 + 'The callback passed to act(...) function ' +
183 + 'must return undefined, or a Promise. You returned %s',
184 + result,
185 + );
186 + }
187 + }
188 +
189 + // flush effects until none remain, and cleanup
190 + try {
191 + if (
192 + actingUpdatesScopeDepth === 1 &&
193 + (isSchedulerMocked === false || previousIsSomeRendererActing === false)
194 + ) {
195 + // we're about to exit the act() scope,
196 + // now's the time to flush effects
197 + flushWork();
198 + }
199 + onDone();
200 + } catch (err) {
201 + onDone();
202 + throw err;
203 + }
204 +
205 + // in the sync case, the returned thenable only warns *if* await-ed
206 + return {
207 + then(resolve) {
208 + if (__DEV__) {
209 + console.error(
210 + 'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
211 + );
212 + }
213 + resolve();
214 + },
215 + };
216 + }
217 +}