@samitouri / QOS-React / commits / a4ecd85e86

act: Batch updates, even in legacy roots (#21797)

In legacy roots, if an update originates outside of `batchedUpdates`, check if it's inside an `act` scope; if so, treat it as if it were batched. This is only necessary in legacy roots because in concurrent roots, updates are batched by default. With this change, the Test Utils and Test Renderer versions of `act` are nothing more than aliases of the isomorphic API (still not exposed, but will likely be the recommended API that replaces the others).

Andrew Clark committed Jul 12, 2021 at 20:15 UTC a4ecd85e8628d1fdfe77c5615934de40af7d9537
8 files changed +100 -19
packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
+1 -1
@@ -2462,7 +2462,7 @@ describe('InspectedElement', () => {
2462 };
2463 const toggleError = async forceError => {
2464 await withErrorsOrWarningsIgnored(['ErrorBoundary'], async () => {
2465 - await utils.actAsync(() => {
2465 + await TestUtilsAct(() => {
2466 bridge.send('overrideError', {
2467 id: targetErrorBoundaryID,
2468 rendererID: store.getRendererIDForElement(targetErrorBoundaryID),
packages/react-dom/src/test-utils/ReactTestUtils.js
+1 -7
@@ -33,14 +33,8 @@ const getNodeFromInstance = EventInternals[1];
33 const getFiberCurrentPropsFromNode = EventInternals[2];
34 const enqueueStateRestore = EventInternals[3];
35 const restoreStateIfNeeded = EventInternals[4];
36 -const batchedUpdates = EventInternals[5];
36
38 -const act_notBatchedInLegacyMode = React.unstable_act;
39 -function act(callback) {
40 - return act_notBatchedInLegacyMode(() => {
41 - return batchedUpdates(callback);
42 - });
43 -}
37 +const act = React.unstable_act;
38
39 function Event(suffix) {}
40
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+11 -2
@@ -518,7 +518,9 @@ export function scheduleUpdateOnFiber(
518 if (
519 lane === SyncLane &&
520 executionContext === NoContext &&
521 - (fiber.mode & ConcurrentMode) === NoMode
521 + (fiber.mode & ConcurrentMode) === NoMode &&
522 + // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
523 + !(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
524 ) {
525 // Flush the synchronous work now, unless we're already working or inside
526 // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
@@ -668,6 +670,9 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
670 // Special case: Sync React callbacks are scheduled on a special
671 // internal queue
672 if (root.tag === LegacyRoot) {
673 + if (__DEV__ && ReactCurrentActQueue.isBatchingLegacy !== null) {
674 + ReactCurrentActQueue.didScheduleLegacyUpdate = true;
675 + }
676 scheduleLegacySyncCallback(performSyncWorkOnRoot.bind(null, root));
677 } else {
678 scheduleSyncCallback(performSyncWorkOnRoot.bind(null, root));
@@ -1049,7 +1054,11 @@ export function batchedUpdates<A, R>(fn: A => R, a: A): R {
1054 executionContext = prevExecutionContext;
1055 // If there were legacy sync updates, flush them at the end of the outer
1056 // most batchedUpdates-like method.
1052 - if (executionContext === NoContext) {
1057 + if (
1058 + executionContext === NoContext &&
1059 + // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
1060 + !(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
1061 + ) {
1062 resetRenderTimer();
1063 flushSyncCallbacksOnlyInLegacyMode();
1064 }
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+11 -2
@@ -518,7 +518,9 @@ export function scheduleUpdateOnFiber(
518 if (
519 lane === SyncLane &&
520 executionContext === NoContext &&
521 - (fiber.mode & ConcurrentMode) === NoMode
521 + (fiber.mode & ConcurrentMode) === NoMode &&
522 + // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
523 + !(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
524 ) {
525 // Flush the synchronous work now, unless we're already working or inside
526 // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
@@ -668,6 +670,9 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
670 // Special case: Sync React callbacks are scheduled on a special
671 // internal queue
672 if (root.tag === LegacyRoot) {
673 + if (__DEV__ && ReactCurrentActQueue.isBatchingLegacy !== null) {
674 + ReactCurrentActQueue.didScheduleLegacyUpdate = true;
675 + }
676 scheduleLegacySyncCallback(performSyncWorkOnRoot.bind(null, root));
677 } else {
678 scheduleSyncCallback(performSyncWorkOnRoot.bind(null, root));
@@ -1049,7 +1054,11 @@ export function batchedUpdates<A, R>(fn: A => R, a: A): R {
1054 executionContext = prevExecutionContext;
1055 // If there were legacy sync updates, flush them at the end of the outer
1056 // most batchedUpdates-like method.
1052 - if (executionContext === NoContext) {
1057 + if (
1058 + executionContext === NoContext &&
1059 + // Treat `act` as if it's inside `batchedUpdates`, even in legacy mode.
1060 + !(__DEV__ && ReactCurrentActQueue.isBatchingLegacy)
1061 + ) {
1062 resetRenderTimer();
1063 flushSyncCallbacksOnlyInLegacyMode();
1064 }
packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js
+49
@@ -78,4 +78,53 @@ describe('isomorphic act()', () => {
78 });
79 expect(returnValue).toEqual('hi');
80 });
81 +
82 + // @gate __DEV__
83 + test('in legacy mode, updates are batched', () => {
84 + const root = ReactNoop.createLegacyRoot();
85 +
86 + // Outside of `act`, legacy updates are flushed completely synchronously
87 + root.render('A');
88 + expect(root).toMatchRenderedOutput('A');
89 +
90 + // `act` will batch the updates and flush them at the end
91 + act(() => {
92 + root.render('B');
93 + // Hasn't flushed yet
94 + expect(root).toMatchRenderedOutput('A');
95 +
96 + // Confirm that a nested `batchedUpdates` call won't cause the updates
97 + // to flush early.
98 + ReactNoop.batchedUpdates(() => {
99 + root.render('C');
100 + });
101 +
102 + // Still hasn't flushed
103 + expect(root).toMatchRenderedOutput('A');
104 + });
105 +
106 + // Now everything renders in a single batch.
107 + expect(root).toMatchRenderedOutput('C');
108 + });
109 +
110 + // @gate __DEV__
111 + test('in legacy mode, in an async scope, updates are batched until the first `await`', async () => {
112 + const root = ReactNoop.createLegacyRoot();
113 +
114 + await act(async () => {
115 + // These updates are batched. This replicates the behavior of the original
116 + // `act` implementation, for compatibility.
117 + root.render('A');
118 + root.render('B');
119 + // Nothing has rendered yet.
120 + expect(root).toMatchRenderedOutput(null);
121 + await null;
122 + // Updates are flushed after the first await.
123 + expect(root).toMatchRenderedOutput('B');
124 +
125 + // Subsequent updates in the same scope aren't batched.
126 + root.render('C');
127 + expect(root).toMatchRenderedOutput('C');
128 + });
129 + });
130 });
packages/react-test-renderer/src/ReactTestRenderer.js
+1 -7
@@ -7,7 +7,6 @@
7 * @flow
8 */
9
10 -import type {Thenable} from 'shared/ReactTypes';
10 import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
11 import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
12 import type {Instance, TextInstance} from './ReactTestHostConfig';
@@ -50,12 +49,7 @@ import {getPublicInstance} from './ReactTestHostConfig';
49 import {ConcurrentRoot, LegacyRoot} from 'react-reconciler/src/ReactRootTags';
50 import {allowConcurrentByDefault} from 'shared/ReactFeatureFlags';
51
53 -const act_notBatchedInLegacyMode = React.unstable_act;
54 -function act<T>(callback: () => T): Thenable<T> {
55 - return act_notBatchedInLegacyMode(() => {
56 - return batchedUpdates(callback);
57 - });
58 -}
52 +const act = React.unstable_act;
53
54 // TODO: Remove from public bundle
55
packages/react/src/ReactAct.js
+22
@@ -28,12 +28,34 @@ export function act<T>(callback: () => T | Thenable<T>): Thenable<T> {
28 ReactCurrentActQueue.current = [];
29 }
30
31 + const prevIsBatchingLegacy = ReactCurrentActQueue.isBatchingLegacy;
32 let result;
33 try {
34 + // Used to reproduce behavior of `batchedUpdates` in legacy mode. Only
35 + // set to `true` while the given callback is executed, not for updates
36 + // triggered during an async event, because this is how the legacy
37 + // implementation of `act` behaved.
38 + ReactCurrentActQueue.isBatchingLegacy = true;
39 result = callback();
40 +
41 + // Replicate behavior of original `act` implementation in legacy mode,
42 + // which flushed updates immediately after the scope function exits, even
43 + // if it's an async function.
44 + if (
45 + !prevIsBatchingLegacy &&
46 + ReactCurrentActQueue.didScheduleLegacyUpdate
47 + ) {
48 + const queue = ReactCurrentActQueue.current;
49 + if (queue !== null) {
50 + ReactCurrentActQueue.didScheduleLegacyUpdate = false;
51 + flushActQueue(queue);
52 + }
53 + }
54 } catch (error) {
55 popActScope(prevActScopeDepth);
56 throw error;
57 + } finally {
58 + ReactCurrentActQueue.isBatchingLegacy = prevIsBatchingLegacy;
59 }
60
61 if (
packages/react/src/ReactCurrentActQueue.js
+4
@@ -17,6 +17,10 @@ const ReactCurrentActQueue = {
17 // on at the testing frameworks layer? Instead of what we do now, which
18 // is check if a `jest` global is defined.
19 disableActWarning: (false: boolean),
20 +
21 + // Used to reproduce behavior of `batchedUpdates` in legacy mode.
22 + isBatchingLegacy: false,
23 + didScheduleLegacyUpdate: false,
24 };
25
26 export default ReactCurrentActQueue;