@samitouri / QOS-React-2 / commits / 54e88ed12c

Bugfix: Flush legacy sync passive effects at beginning of event (#21846)

* Re-land recent flushSync changes Adds back #21776 and #21775, which were removed due to an internal e2e test failure. Will attempt to fix in subsequent commits. * Failing test: Legacy mode sync passive effects In concurrent roots, if a render is synchronous, we flush its passive effects synchronously. In legacy roots, we don't do this because all updates are synchronous — so we need to flush at the beginning of the next event. This is how `discreteUpdates` worked. * Flush legacy passive effects at beginning of event Fixes test added in previous commit.

Andrew Clark committed Jul 10, 2021 at 14:15 UTC 54e88ed12c931fa717738849ba5656cc1cf3527a
17 files changed +184 -337
packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap
+14 -14
@@ -40,7 +40,7 @@ Object {
40 6 => 1,
41 },
42 "passiveEffectDuration": null,
43 - "priorityLevel": "Normal",
43 + "priorityLevel": "Immediate",
44 "timestamp": 16,
45 "updaters": Array [
46 Object {
@@ -87,7 +87,7 @@ Object {
87 4 => 2,
88 },
89 "passiveEffectDuration": null,
90 - "priorityLevel": "Normal",
90 + "priorityLevel": "Immediate",
91 "timestamp": 15,
92 "updaters": Array [
93 Object {
@@ -186,7 +186,7 @@ Object {
186 6 => 1,
187 },
188 "passiveEffectDuration": null,
189 - "priorityLevel": "Normal",
189 + "priorityLevel": "Immediate",
190 "timestamp": 12,
191 "updaters": Array [
192 Object {
@@ -445,7 +445,7 @@ Object {
445 ],
446 ],
447 "passiveEffectDuration": null,
448 - "priorityLevel": "Normal",
448 + "priorityLevel": "Immediate",
449 "timestamp": 12,
450 "updaters": Array [
451 Object {
@@ -938,7 +938,7 @@ Object {
938 ],
939 ],
940 "passiveEffectDuration": null,
941 - "priorityLevel": "Normal",
941 + "priorityLevel": "Immediate",
942 "timestamp": 11,
943 "updaters": Array [
944 Object {
@@ -1597,7 +1597,7 @@ Object {
1597 17 => 1,
1598 },
1599 "passiveEffectDuration": null,
1600 - "priorityLevel": "Normal",
1600 + "priorityLevel": "Immediate",
1601 "timestamp": 24,
1602 "updaters": Array [
1603 Object {
@@ -1687,7 +1687,7 @@ Object {
1687 "fiberActualDurations": Map {},
1688 "fiberSelfDurations": Map {},
1689 "passiveEffectDuration": 0,
1690 - "priorityLevel": "Normal",
1690 + "priorityLevel": "Immediate",
1691 "timestamp": 34,
1692 "updaters": Array [
1693 Object {
@@ -2223,7 +2223,7 @@ Object {
2223 ],
2224 ],
2225 "passiveEffectDuration": null,
2226 - "priorityLevel": "Normal",
2226 + "priorityLevel": "Immediate",
2227 "timestamp": 24,
2228 "updaters": Array [
2229 Object {
@@ -2310,7 +2310,7 @@ Object {
2310 "fiberActualDurations": Array [],
2311 "fiberSelfDurations": Array [],
2312 "passiveEffectDuration": 0,
2313 - "priorityLevel": "Normal",
2313 + "priorityLevel": "Immediate",
2314 "timestamp": 34,
2315 "updaters": Array [
2316 Object {
@@ -2431,7 +2431,7 @@ Object {
2431 2 => 0,
2432 },
2433 "passiveEffectDuration": null,
2434 - "priorityLevel": "Normal",
2434 + "priorityLevel": "Immediate",
2435 "timestamp": 0,
2436 "updaters": Array [
2437 Object {
@@ -2506,7 +2506,7 @@ Object {
2506 3 => 0,
2507 },
2508 "passiveEffectDuration": 0,
2509 - "priorityLevel": "Normal",
2509 + "priorityLevel": "Immediate",
2510 "timestamp": 0,
2511 "updaters": Array [
2512 Object {
@@ -2715,7 +2715,7 @@ Object {
2715 ],
2716 ],
2717 "passiveEffectDuration": 0,
2718 - "priorityLevel": "Normal",
2718 + "priorityLevel": "Immediate",
2719 "timestamp": 0,
2720 "updaters": Array [
2721 Object {
@@ -3071,7 +3071,7 @@ Object {
3071 7 => 0,
3072 },
3073 "passiveEffectDuration": null,
3074 - "priorityLevel": "Normal",
3074 + "priorityLevel": "Immediate",
3075 "timestamp": 0,
3076 "updaters": Array [
3077 Object {
@@ -3515,7 +3515,7 @@ Object {
3515 ],
3516 ],
3517 "passiveEffectDuration": null,
3518 - "priorityLevel": "Normal",
3518 + "priorityLevel": "Immediate",
3519 "timestamp": 0,
3520 "updaters": Array [
3521 Object {
packages/react-dom/src/__tests__/ReactDOMFiber-test.js
+1 -7
@@ -1154,19 +1154,13 @@ describe('ReactDOMFiber', () => {
1154 expect(ops).toEqual(['A']);
1155
1156 if (__DEV__) {
1157 - const errorCalls = console.error.calls.count();
1157 + expect(console.error.calls.count()).toBe(2);
1158 expect(console.error.calls.argsFor(0)[0]).toMatch(
1159 'ReactDOM.render is no longer supported in React 18',
1160 );
1161 expect(console.error.calls.argsFor(1)[0]).toMatch(
1162 'ReactDOM.render is no longer supported in React 18',
1163 );
1164 - // TODO: this warning shouldn't be firing in the first place if user didn't call it.
1165 - for (let i = 2; i < errorCalls; i++) {
1166 - expect(console.error.calls.argsFor(i)[0]).toMatch(
1167 - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.',
1168 - );
1169 - }
1164 }
1165 });
1166
packages/react-dom/src/__tests__/ReactMount-test.js
+6 -6
@@ -277,7 +277,7 @@ describe('ReactMount', () => {
277 expect(calls).toBe(5);
278 });
279
280 - it('initial mount is sync inside batchedUpdates, but task work is deferred until the end of the batch', () => {
280 + it('initial mount of legacy root is sync inside batchedUpdates, as if it were wrapped in flushSync', () => {
281 const container1 = document.createElement('div');
282 const container2 = document.createElement('div');
283
@@ -302,12 +302,12 @@ describe('ReactMount', () => {
302
303 // Initial mount on another root. Should flush immediately.
304 ReactDOM.render(<Foo>a</Foo>, container2);
305 - // The update did not flush yet.
306 - expect(container1.textContent).toEqual('1');
307 - // The initial mount flushed, but not the update scheduled in cDM.
308 - expect(container2.textContent).toEqual('a');
305 + // The earlier update also flushed, since flushSync flushes all pending
306 + // sync work across all roots.
307 + expect(container1.textContent).toEqual('2');
308 + // Layout updates are also flushed synchronously
309 + expect(container2.textContent).toEqual('a!');
310 });
310 - // All updates have flushed.
311 expect(container1.textContent).toEqual('2');
312 expect(container2.textContent).toEqual('a!');
313 });
packages/react-dom/src/client/ReactDOM.js
+2 -2
@@ -23,8 +23,8 @@ import {createEventHandle} from './ReactDOMEventHandle';
23 import {
24 batchedUpdates,
25 discreteUpdates,
26 - flushDiscreteUpdates,
26 flushSync,
27 + flushSyncWithoutWarningIfAlreadyRendering,
28 flushControlled,
29 injectIntoDevTools,
30 attemptSynchronousHydration,
@@ -100,7 +100,7 @@ setRestoreImplementation(restoreControlledState);
100 setBatchingImplementation(
101 batchedUpdates,
102 discreteUpdates,
103 - flushDiscreteUpdates,
103 + flushSyncWithoutWarningIfAlreadyRendering,
104 );
105
106 function createPortal(
packages/react-dom/src/client/ReactDOMLegacy.js
+3 -3
@@ -29,7 +29,7 @@ import {
29 createContainer,
30 findHostInstanceWithNoPortals,
31 updateContainer,
32 - unbatchedUpdates,
32 + flushSyncWithoutWarningIfAlreadyRendering,
33 getPublicRootInstance,
34 findHostInstance,
35 findHostInstanceWithWarning,
@@ -174,7 +174,7 @@ function legacyRenderSubtreeIntoContainer(
174 };
175 }
176 // Initial mount should not be batched.
177 - unbatchedUpdates(() => {
177 + flushSyncWithoutWarningIfAlreadyRendering(() => {
178 updateContainer(children, fiberRoot, parentComponent, callback);
179 });
180 } else {
@@ -357,7 +357,7 @@ export function unmountComponentAtNode(container: Container) {
357 }
358
359 // Unmount should not be batched.
360 - unbatchedUpdates(() => {
360 + flushSyncWithoutWarningIfAlreadyRendering(() => {
361 legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
362 // $FlowFixMe This should probably use `delete container._reactRootContainer`
363 container._reactRootContainer = null;
packages/react-dom/src/events/ReactDOMUpdateBatching.js
+4 -4
@@ -23,7 +23,7 @@ let batchedUpdatesImpl = function(fn, bookkeeping) {
23 let discreteUpdatesImpl = function(fn, a, b, c, d) {
24 return fn(a, b, c, d);
25 };
26 -let flushDiscreteUpdatesImpl = function() {};
26 +let flushSyncImpl = function() {};
27
28 let isInsideEventHandler = false;
29
@@ -39,7 +39,7 @@ function finishEventHandler() {
39 // bails out of the update without touching the DOM.
40 // TODO: Restore state in the microtask, after the discrete updates flush,
41 // instead of early flushing them here.
42 - flushDiscreteUpdatesImpl();
42 + flushSyncImpl();
43 restoreStateIfNeeded();
44 }
45 }
@@ -67,9 +67,9 @@ export function discreteUpdates(fn, a, b, c, d) {
67 export function setBatchingImplementation(
68 _batchedUpdatesImpl,
69 _discreteUpdatesImpl,
70 - _flushDiscreteUpdatesImpl,
70 + _flushSyncImpl,
71 ) {
72 batchedUpdatesImpl = _batchedUpdatesImpl;
73 discreteUpdatesImpl = _discreteUpdatesImpl;
74 - flushDiscreteUpdatesImpl = _flushDiscreteUpdatesImpl;
74 + flushSyncImpl = _flushSyncImpl;
75 }
packages/react-noop-renderer/src/ReactNoop.js
-2
@@ -38,10 +38,8 @@ export const {
38 flushExpired,
39 batchedUpdates,
40 deferredUpdates,
41 - unbatchedUpdates,
41 discreteUpdates,
42 idleUpdates,
44 - flushDiscreteUpdates,
43 flushSync,
44 flushPassiveEffects,
45 act,
packages/react-noop-renderer/src/ReactNoopPersistent.js
-1
@@ -38,7 +38,6 @@ export const {
38 flushExpired,
39 batchedUpdates,
40 deferredUpdates,
41 - unbatchedUpdates,
41 discreteUpdates,
42 idleUpdates,
43 flushDiscreteUpdates,
packages/react-noop-renderer/src/createReactNoop.js
-4
@@ -901,8 +901,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
901
902 deferredUpdates: NoopRenderer.deferredUpdates,
903
904 - unbatchedUpdates: NoopRenderer.unbatchedUpdates,
905 -
904 discreteUpdates: NoopRenderer.discreteUpdates,
905
906 idleUpdates<T>(fn: () => T): T {
@@ -915,8 +913,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
913 }
914 },
915
918 - flushDiscreteUpdates: NoopRenderer.flushDiscreteUpdates,
919 -
916 flushSync(fn: () => mixed) {
917 NoopRenderer.flushSync(fn);
918 },
packages/react-reconciler/src/ReactFiberReconciler.js
+5 -10
@@ -18,12 +18,11 @@ import {
18 createContainer as createContainer_old,
19 updateContainer as updateContainer_old,
20 batchedUpdates as batchedUpdates_old,
21 - unbatchedUpdates as unbatchedUpdates_old,
21 deferredUpdates as deferredUpdates_old,
22 discreteUpdates as discreteUpdates_old,
24 - flushDiscreteUpdates as flushDiscreteUpdates_old,
23 flushControlled as flushControlled_old,
24 flushSync as flushSync_old,
25 + flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_old,
26 flushPassiveEffects as flushPassiveEffects_old,
27 getPublicRootInstance as getPublicRootInstance_old,
28 attemptSynchronousHydration as attemptSynchronousHydration_old,
@@ -56,12 +55,11 @@ import {
55 createContainer as createContainer_new,
56 updateContainer as updateContainer_new,
57 batchedUpdates as batchedUpdates_new,
59 - unbatchedUpdates as unbatchedUpdates_new,
58 deferredUpdates as deferredUpdates_new,
59 discreteUpdates as discreteUpdates_new,
62 - flushDiscreteUpdates as flushDiscreteUpdates_new,
60 flushControlled as flushControlled_new,
61 flushSync as flushSync_new,
62 + flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_new,
63 flushPassiveEffects as flushPassiveEffects_new,
64 getPublicRootInstance as getPublicRootInstance_new,
65 attemptSynchronousHydration as attemptSynchronousHydration_new,
@@ -99,22 +97,19 @@ export const updateContainer = enableNewReconciler
97 export const batchedUpdates = enableNewReconciler
98 ? batchedUpdates_new
99 : batchedUpdates_old;
102 -export const unbatchedUpdates = enableNewReconciler
103 - ? unbatchedUpdates_new
104 - : unbatchedUpdates_old;
100 export const deferredUpdates = enableNewReconciler
101 ? deferredUpdates_new
102 : deferredUpdates_old;
103 export const discreteUpdates = enableNewReconciler
104 ? discreteUpdates_new
105 : discreteUpdates_old;
111 -export const flushDiscreteUpdates = enableNewReconciler
112 - ? flushDiscreteUpdates_new
113 - : flushDiscreteUpdates_old;
106 export const flushControlled = enableNewReconciler
107 ? flushControlled_new
108 : flushControlled_old;
109 export const flushSync = enableNewReconciler ? flushSync_new : flushSync_old;
110 +export const flushSyncWithoutWarningIfAlreadyRendering = enableNewReconciler
111 + ? flushSyncWithoutWarningIfAlreadyRendering_new
112 + : flushSyncWithoutWarningIfAlreadyRendering_old;
113 export const flushPassiveEffects = enableNewReconciler
114 ? flushPassiveEffects_new
115 : flushPassiveEffects_old;
packages/react-reconciler/src/ReactFiberReconciler.new.js
+2 -4
@@ -52,12 +52,11 @@ import {
52 scheduleUpdateOnFiber,
53 flushRoot,
54 batchedUpdates,
55 - unbatchedUpdates,
55 flushSync,
56 flushControlled,
57 deferredUpdates,
58 discreteUpdates,
60 - flushDiscreteUpdates,
59 + flushSyncWithoutWarningIfAlreadyRendering,
60 flushPassiveEffects,
61 } from './ReactFiberWorkLoop.new';
62 import {
@@ -327,12 +326,11 @@ export function updateContainer(
326
327 export {
328 batchedUpdates,
330 - unbatchedUpdates,
329 deferredUpdates,
330 discreteUpdates,
333 - flushDiscreteUpdates,
331 flushControlled,
332 flushSync,
333 + flushSyncWithoutWarningIfAlreadyRendering,
334 flushPassiveEffects,
335 };
336
packages/react-reconciler/src/ReactFiberReconciler.old.js
+2 -4
@@ -52,12 +52,11 @@ import {
52 scheduleUpdateOnFiber,
53 flushRoot,
54 batchedUpdates,
55 - unbatchedUpdates,
55 flushSync,
56 flushControlled,
57 deferredUpdates,
58 discreteUpdates,
60 - flushDiscreteUpdates,
59 + flushSyncWithoutWarningIfAlreadyRendering,
60 flushPassiveEffects,
61 } from './ReactFiberWorkLoop.old';
62 import {
@@ -327,12 +326,11 @@ export function updateContainer(
326
327 export {
328 batchedUpdates,
330 - unbatchedUpdates,
329 deferredUpdates,
330 discreteUpdates,
333 - flushDiscreteUpdates,
331 flushControlled,
332 flushSync,
333 + flushSyncWithoutWarningIfAlreadyRendering,
334 flushPassiveEffects,
335 };
336
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+42 -107
@@ -246,12 +246,11 @@ const {
246
247 type ExecutionContext = number;
248
249 -export const NoContext = /* */ 0b00000;
250 -const BatchedContext = /* */ 0b00001;
251 -const LegacyUnbatchedContext = /* */ 0b00010;
252 -const RenderContext = /* */ 0b00100;
253 -const CommitContext = /* */ 0b01000;
254 -export const RetryAfterError = /* */ 0b10000;
249 +export const NoContext = /* */ 0b0000;
250 +const BatchedContext = /* */ 0b0001;
251 +const RenderContext = /* */ 0b0010;
252 +const CommitContext = /* */ 0b0100;
253 +export const RetryAfterError = /* */ 0b1000;
254
255 type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
256 const RootIncomplete = 0;
@@ -515,35 +514,19 @@ export function scheduleUpdateOnFiber(
514 }
515 }
516
518 - if (lane === SyncLane) {
519 - if (
520 - // Check if we're inside unbatchedUpdates
521 - (executionContext & LegacyUnbatchedContext) !== NoContext &&
522 - // Check if we're not already rendering
523 - (executionContext & (RenderContext | CommitContext)) === NoContext
524 - ) {
525 - // This is a legacy edge case. The initial mount of a ReactDOM.render-ed
526 - // root inside of batchedUpdates should be synchronous, but layout updates
527 - // should be deferred until the end of the batch.
528 - performSyncWorkOnRoot(root);
529 - } else {
530 - ensureRootIsScheduled(root, eventTime);
531 - if (
532 - executionContext === NoContext &&
533 - (fiber.mode & ConcurrentMode) === NoMode
534 - ) {
535 - // Flush the synchronous work now, unless we're already working or inside
536 - // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
537 - // scheduleCallbackForFiber to preserve the ability to schedule a callback
538 - // without immediately flushing it. We only do this for user-initiated
539 - // updates, to preserve historical behavior of legacy mode.
540 - resetRenderTimer();
541 - flushSyncCallbacksOnlyInLegacyMode();
542 - }
543 - }
544 - } else {
545 - // Schedule other updates after in case the callback is sync.
546 - ensureRootIsScheduled(root, eventTime);
517 + ensureRootIsScheduled(root, eventTime);
518 + if (
519 + lane === SyncLane &&
520 + executionContext === NoContext &&
521 + (fiber.mode & ConcurrentMode) === NoMode
522 + ) {
523 + // Flush the synchronous work now, unless we're already working or inside
524 + // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
525 + // scheduleCallbackForFiber to preserve the ability to schedule a callback
526 + // without immediately flushing it. We only do this for user-initiated
527 + // updates, to preserve historical behavior of legacy mode.
528 + resetRenderTimer();
529 + flushSyncCallbacksOnlyInLegacyMode();
530 }
531
532 return root;
@@ -1044,34 +1027,6 @@ export function getExecutionContext(): ExecutionContext {
1027 return executionContext;
1028 }
1029
1047 -export function flushDiscreteUpdates() {
1048 - // TODO: Should be able to flush inside batchedUpdates, but not inside `act`.
1049 - // However, `act` uses `batchedUpdates`, so there's no way to distinguish
1050 - // those two cases. Need to fix this before exposing flushDiscreteUpdates
1051 - // as a public API.
1052 - if (
1053 - (executionContext & (BatchedContext | RenderContext | CommitContext)) !==
1054 - NoContext
1055 - ) {
1056 - if (__DEV__) {
1057 - if ((executionContext & RenderContext) !== NoContext) {
1058 - console.error(
1059 - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is ' +
1060 - 'already rendering.',
1061 - );
1062 - }
1063 - }
1064 - // We're already rendering, so we can't synchronously flush pending work.
1065 - // This is probably a nested event dispatch triggered by a lifecycle/effect,
1066 - // like `el.focus()`. Exit.
1067 - return;
1068 - }
1069 - flushSyncCallbacks();
1070 - // If the discrete updates scheduled passive effects, flush them now so that
1071 - // they fire before the next serial event.
1072 - flushPassiveEffects();
1073 -}
1074 -
1030 export function deferredUpdates<A>(fn: () => A): A {
1031 const previousPriority = getCurrentUpdatePriority();
1032 const prevTransition = ReactCurrentBatchConfig.transition;
@@ -1123,26 +1078,19 @@ export function discreteUpdates<A, B, C, D, R>(
1078 }
1079 }
1080
1126 -export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
1127 - const prevExecutionContext = executionContext;
1128 - executionContext &= ~BatchedContext;
1129 - executionContext |= LegacyUnbatchedContext;
1130 - try {
1131 - return fn(a);
1132 - } finally {
1133 - executionContext = prevExecutionContext;
1134 - // If there were legacy sync updates, flush them at the end of the outer
1135 - // most batchedUpdates-like method.
1136 - if (executionContext === NoContext) {
1137 - resetRenderTimer();
1138 - // TODO: I think this call is redundant, because we flush inside
1139 - // scheduleUpdateOnFiber when LegacyUnbatchedContext is set.
1140 - flushSyncCallbacksOnlyInLegacyMode();
1141 - }
1081 +export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
1082 + fn: A => R,
1083 + a: A,
1084 +): R {
1085 + // In legacy mode, we flush pending passive effects at the beginning of the
1086 + // next event, not at the end of the previous one.
1087 + if (
1088 + rootWithPendingPassiveEffects !== null &&
1089 + rootWithPendingPassiveEffects.tag === LegacyRoot
1090 + ) {
1091 + flushPassiveEffects();
1092 }
1143 -}
1093
1145 -export function flushSync<A, R>(fn: A => R, a: A): R {
1094 const prevExecutionContext = executionContext;
1095 executionContext |= BatchedContext;
1096
@@ -1165,18 +1113,23 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
1113 // the stack.
1114 if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1115 flushSyncCallbacks();
1168 - } else {
1169 - if (__DEV__) {
1170 - console.error(
1171 - 'flushSync was called from inside a lifecycle method. React cannot ' +
1172 - 'flush when React is already rendering. Consider moving this call to ' +
1173 - 'a scheduler task or micro task.',
1174 - );
1175 - }
1116 }
1117 }
1118 }
1119
1120 +export function flushSync<A, R>(fn: A => R, a: A): R {
1121 + if (__DEV__) {
1122 + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
1123 + console.error(
1124 + 'flushSync was called from inside a lifecycle method. React cannot ' +
1125 + 'flush when React is already rendering. Consider moving this call to ' +
1126 + 'a scheduler task or micro task.',
1127 + );
1128 + }
1129 + }
1130 + return flushSyncWithoutWarningIfAlreadyRendering(fn, a);
1131 +}
1132 +
1133 export function flushControlled(fn: () => mixed): void {
1134 const prevExecutionContext = executionContext;
1135 executionContext |= BatchedContext;
@@ -1974,24 +1927,6 @@ function commitRootImpl(root, renderPriorityLevel) {
1927 throw error;
1928 }
1929
1977 - if ((executionContext & LegacyUnbatchedContext) !== NoContext) {
1978 - if (__DEV__) {
1979 - if (enableDebugTracing) {
1980 - logCommitStopped();
1981 - }
1982 - }
1983 -
1984 - if (enableSchedulingProfiler) {
1985 - markCommitStopped();
1986 - }
1987 -
1988 - // This is a legacy edge case. We just committed the initial mount of
1989 - // a ReactDOM.render-ed root inside of batchedUpdates. The commit fired
1990 - // synchronously, but layout updates should be deferred until the end
1991 - // of the batch.
1992 - return null;
1993 - }
1994 -
1930 // If the passive effects are the result of a discrete render, flush them
1931 // synchronously at the end of the current task so that the result is
1932 // immediately observable. Otherwise, we assume that they are not
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+42 -107
@@ -246,12 +246,11 @@ const {
246
247 type ExecutionContext = number;
248
249 -export const NoContext = /* */ 0b00000;
250 -const BatchedContext = /* */ 0b00001;
251 -const LegacyUnbatchedContext = /* */ 0b00010;
252 -const RenderContext = /* */ 0b00100;
253 -const CommitContext = /* */ 0b01000;
254 -export const RetryAfterError = /* */ 0b10000;
249 +export const NoContext = /* */ 0b0000;
250 +const BatchedContext = /* */ 0b0001;
251 +const RenderContext = /* */ 0b0010;
252 +const CommitContext = /* */ 0b0100;
253 +export const RetryAfterError = /* */ 0b1000;
254
255 type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
256 const RootIncomplete = 0;
@@ -515,35 +514,19 @@ export function scheduleUpdateOnFiber(
514 }
515 }
516
518 - if (lane === SyncLane) {
519 - if (
520 - // Check if we're inside unbatchedUpdates
521 - (executionContext & LegacyUnbatchedContext) !== NoContext &&
522 - // Check if we're not already rendering
523 - (executionContext & (RenderContext | CommitContext)) === NoContext
524 - ) {
525 - // This is a legacy edge case. The initial mount of a ReactDOM.render-ed
526 - // root inside of batchedUpdates should be synchronous, but layout updates
527 - // should be deferred until the end of the batch.
528 - performSyncWorkOnRoot(root);
529 - } else {
530 - ensureRootIsScheduled(root, eventTime);
531 - if (
532 - executionContext === NoContext &&
533 - (fiber.mode & ConcurrentMode) === NoMode
534 - ) {
535 - // Flush the synchronous work now, unless we're already working or inside
536 - // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
537 - // scheduleCallbackForFiber to preserve the ability to schedule a callback
538 - // without immediately flushing it. We only do this for user-initiated
539 - // updates, to preserve historical behavior of legacy mode.
540 - resetRenderTimer();
541 - flushSyncCallbacksOnlyInLegacyMode();
542 - }
543 - }
544 - } else {
545 - // Schedule other updates after in case the callback is sync.
546 - ensureRootIsScheduled(root, eventTime);
517 + ensureRootIsScheduled(root, eventTime);
518 + if (
519 + lane === SyncLane &&
520 + executionContext === NoContext &&
521 + (fiber.mode & ConcurrentMode) === NoMode
522 + ) {
523 + // Flush the synchronous work now, unless we're already working or inside
524 + // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
525 + // scheduleCallbackForFiber to preserve the ability to schedule a callback
526 + // without immediately flushing it. We only do this for user-initiated
527 + // updates, to preserve historical behavior of legacy mode.
528 + resetRenderTimer();
529 + flushSyncCallbacksOnlyInLegacyMode();
530 }
531
532 return root;
@@ -1044,34 +1027,6 @@ export function getExecutionContext(): ExecutionContext {
1027 return executionContext;
1028 }
1029
1047 -export function flushDiscreteUpdates() {
1048 - // TODO: Should be able to flush inside batchedUpdates, but not inside `act`.
1049 - // However, `act` uses `batchedUpdates`, so there's no way to distinguish
1050 - // those two cases. Need to fix this before exposing flushDiscreteUpdates
1051 - // as a public API.
1052 - if (
1053 - (executionContext & (BatchedContext | RenderContext | CommitContext)) !==
1054 - NoContext
1055 - ) {
1056 - if (__DEV__) {
1057 - if ((executionContext & RenderContext) !== NoContext) {
1058 - console.error(
1059 - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is ' +
1060 - 'already rendering.',
1061 - );
1062 - }
1063 - }
1064 - // We're already rendering, so we can't synchronously flush pending work.
1065 - // This is probably a nested event dispatch triggered by a lifecycle/effect,
1066 - // like `el.focus()`. Exit.
1067 - return;
1068 - }
1069 - flushSyncCallbacks();
1070 - // If the discrete updates scheduled passive effects, flush them now so that
1071 - // they fire before the next serial event.
1072 - flushPassiveEffects();
1073 -}
1074 -
1030 export function deferredUpdates<A>(fn: () => A): A {
1031 const previousPriority = getCurrentUpdatePriority();
1032 const prevTransition = ReactCurrentBatchConfig.transition;
@@ -1123,26 +1078,19 @@ export function discreteUpdates<A, B, C, D, R>(
1078 }
1079 }
1080
1126 -export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
1127 - const prevExecutionContext = executionContext;
1128 - executionContext &= ~BatchedContext;
1129 - executionContext |= LegacyUnbatchedContext;
1130 - try {
1131 - return fn(a);
1132 - } finally {
1133 - executionContext = prevExecutionContext;
1134 - // If there were legacy sync updates, flush them at the end of the outer
1135 - // most batchedUpdates-like method.
1136 - if (executionContext === NoContext) {
1137 - resetRenderTimer();
1138 - // TODO: I think this call is redundant, because we flush inside
1139 - // scheduleUpdateOnFiber when LegacyUnbatchedContext is set.
1140 - flushSyncCallbacksOnlyInLegacyMode();
1141 - }
1081 +export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
1082 + fn: A => R,
1083 + a: A,
1084 +): R {
1085 + // In legacy mode, we flush pending passive effects at the beginning of the
1086 + // next event, not at the end of the previous one.
1087 + if (
1088 + rootWithPendingPassiveEffects !== null &&
1089 + rootWithPendingPassiveEffects.tag === LegacyRoot
1090 + ) {
1091 + flushPassiveEffects();
1092 }
1143 -}
1093
1145 -export function flushSync<A, R>(fn: A => R, a: A): R {
1094 const prevExecutionContext = executionContext;
1095 executionContext |= BatchedContext;
1096
@@ -1165,18 +1113,23 @@ export function flushSync<A, R>(fn: A => R, a: A): R {
1113 // the stack.
1114 if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
1115 flushSyncCallbacks();
1168 - } else {
1169 - if (__DEV__) {
1170 - console.error(
1171 - 'flushSync was called from inside a lifecycle method. React cannot ' +
1172 - 'flush when React is already rendering. Consider moving this call to ' +
1173 - 'a scheduler task or micro task.',
1174 - );
1175 - }
1116 }
1117 }
1118 }
1119
1120 +export function flushSync<A, R>(fn: A => R, a: A): R {
1121 + if (__DEV__) {
1122 + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
1123 + console.error(
1124 + 'flushSync was called from inside a lifecycle method. React cannot ' +
1125 + 'flush when React is already rendering. Consider moving this call to ' +
1126 + 'a scheduler task or micro task.',
1127 + );
1128 + }
1129 + }
1130 + return flushSyncWithoutWarningIfAlreadyRendering(fn, a);
1131 +}
1132 +
1133 export function flushControlled(fn: () => mixed): void {
1134 const prevExecutionContext = executionContext;
1135 executionContext |= BatchedContext;
@@ -1974,24 +1927,6 @@ function commitRootImpl(root, renderPriorityLevel) {
1927 throw error;
1928 }
1929
1977 - if ((executionContext & LegacyUnbatchedContext) !== NoContext) {
1978 - if (__DEV__) {
1979 - if (enableDebugTracing) {
1980 - logCommitStopped();
1981 - }
1982 - }
1983 -
1984 - if (enableSchedulingProfiler) {
1985 - markCommitStopped();
1986 - }
1987 -
1988 - // This is a legacy edge case. We just committed the initial mount of
1989 - // a ReactDOM.render-ed root inside of batchedUpdates. The commit fired
1990 - // synchronously, but layout updates should be deferred until the end
1991 - // of the batch.
1992 - return null;
1993 - }
1994 -
1930 // If the passive effects are the result of a discrete render, flush them
1931 // synchronously at the end of the current task so that the result is
1932 // immediately observable. Otherwise, we assume that they are not
packages/react-reconciler/src/__tests__/ReactFlushSync-test.js
+58 -1
@@ -128,7 +128,7 @@ describe('ReactFlushSync', () => {
128 });
129 });
130
131 - test('do not flush passive effects synchronously in legacy mode', async () => {
131 + test('do not flush passive effects synchronously after render in legacy mode', async () => {
132 function App() {
133 useEffect(() => {
134 Scheduler.unstable_yieldValue('Effect');
@@ -152,6 +152,40 @@ describe('ReactFlushSync', () => {
152 expect(Scheduler).toHaveYielded(['Effect']);
153 });
154
155 + test('flush pending passive effects before scope is called in legacy mode', async () => {
156 + let currentStep = 0;
157 +
158 + function App({step}) {
159 + useEffect(() => {
160 + currentStep = step;
161 + Scheduler.unstable_yieldValue('Effect: ' + step);
162 + }, [step]);
163 + return <Text text={step} />;
164 + }
165 +
166 + const root = ReactNoop.createLegacyRoot();
167 + await act(async () => {
168 + ReactNoop.flushSync(() => {
169 + root.render(<App step={1} />);
170 + });
171 + expect(Scheduler).toHaveYielded([
172 + 1,
173 + // Because we're in legacy mode, we shouldn't have flushed the passive
174 + // effects yet.
175 + ]);
176 + expect(root).toMatchRenderedOutput('1');
177 +
178 + ReactNoop.flushSync(() => {
179 + // This should render step 2 because the passive effect has already
180 + // fired, before the scope function is called.
181 + root.render(<App step={currentStep + 1} />);
182 + });
183 + expect(Scheduler).toHaveYielded(['Effect: 1', 2]);
184 + expect(root).toMatchRenderedOutput('2');
185 + });
186 + expect(Scheduler).toHaveYielded(['Effect: 2']);
187 + });
188 +
189 test("do not flush passive effects synchronously when they aren't the result of a sync render", async () => {
190 function App() {
191 useEffect(() => {
@@ -173,4 +207,27 @@ describe('ReactFlushSync', () => {
207 // Effect flushes after paint.
208 expect(Scheduler).toHaveYielded(['Effect']);
209 });
210 +
211 + test('does not flush pending passive effects', async () => {
212 + function App() {
213 + useEffect(() => {
214 + Scheduler.unstable_yieldValue('Effect');
215 + }, []);
216 + return <Text text="Child" />;
217 + }
218 +
219 + const root = ReactNoop.createRoot();
220 + await act(async () => {
221 + root.render(<App />);
222 + expect(Scheduler).toFlushUntilNextPaint(['Child']);
223 + expect(root).toMatchRenderedOutput('Child');
224 +
225 + // Passive effects are pending. Calling flushSync should not affect them.
226 + ReactNoop.flushSync();
227 + // Effects still haven't fired.
228 + expect(Scheduler).toHaveYielded([]);
229 + });
230 + // Now the effects have fired.
231 + expect(Scheduler).toHaveYielded(['Effect']);
232 + });
233 });
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
+3 -2
@@ -1795,10 +1795,11 @@ describe('ReactHooksWithNoopRenderer', () => {
1795 return <Text text={'Count: ' + count} />;
1796 }
1797 await act(async () => {
1798 - ReactNoop.renderLegacySyncRoot(<Counter count={0} />);
1798 + ReactNoop.flushSync(() => {
1799 + ReactNoop.renderLegacySyncRoot(<Counter count={0} />);
1800 + });
1801
1802 // Even in legacy mode, effects are deferred until after paint
1801 - ReactNoop.flushSync();
1803 expect(Scheduler).toHaveYielded(['Count: (empty)']);
1804 expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
1805 });
packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js
-59
@@ -349,63 +349,4 @@ describe('ReactIncrementalScheduling', () => {
349 // The updates should all be flushed with Task priority
350 expect(ReactNoop).toMatchRenderedOutput(<span prop={5} />);
351 });
352 -
353 - it('can opt-out of batching using unbatchedUpdates', () => {
354 - ReactNoop.flushSync(() => {
355 - ReactNoop.render(<span prop={0} />);
356 - expect(ReactNoop.getChildren()).toEqual([]);
357 - // Should not have flushed yet because we're still batching
358 -
359 - // unbatchedUpdates reverses the effect of batchedUpdates, so sync
360 - // updates are not batched
361 - ReactNoop.unbatchedUpdates(() => {
362 - ReactNoop.render(<span prop={1} />);
363 - expect(ReactNoop).toMatchRenderedOutput(<span prop={1} />);
364 - ReactNoop.render(<span prop={2} />);
365 - expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);
366 - });
367 -
368 - ReactNoop.render(<span prop={3} />);
369 - expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);
370 - });
371 - // Remaining update is now flushed
372 - expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
373 - });
374 -
375 - it('nested updates are always deferred, even inside unbatchedUpdates', () => {
376 - let instance;
377 - class Foo extends React.Component {
378 - state = {step: 0};
379 - componentDidUpdate() {
380 - Scheduler.unstable_yieldValue('componentDidUpdate: ' + this.state.step);
381 - if (this.state.step === 1) {
382 - ReactNoop.unbatchedUpdates(() => {
383 - // This is a nested state update, so it should not be
384 - // flushed synchronously, even though we wrapped it
385 - // in unbatchedUpdates.
386 - this.setState({step: 2});
387 - });
388 - expect(Scheduler).toHaveYielded([
389 - 'render: 1',
390 - 'componentDidUpdate: 1',
391 - ]);
392 - expect(ReactNoop).toMatchRenderedOutput(<span prop={1} />);
393 - }
394 - }
395 - render() {
396 - Scheduler.unstable_yieldValue('render: ' + this.state.step);
397 - instance = this;
398 - return <span prop={this.state.step} />;
399 - }
400 - }
401 - ReactNoop.render(<Foo />);
402 - expect(Scheduler).toFlushAndYield(['render: 0']);
403 - expect(ReactNoop).toMatchRenderedOutput(<span prop={0} />);
404 -
405 - ReactNoop.flushSync(() => {
406 - instance.setState({step: 1});
407 - });
408 - expect(Scheduler).toHaveYielded(['render: 2', 'componentDidUpdate: 2']);
409 - expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);
410 - });
352 });