Replace unbatchedUpdates with flushSync (#21776)

There's a weird quirk leftover from the old Stack (pre-Fiber) implementation where the initial mount of a leagcy (ReactDOM.render) root is flushed synchronously even inside `batchedUpdates`. The original workaround for this was an internal method called `unbatchedUpdates`. We've since added another API that works almost the same way, `flushSync`. The only difference is that `unbatchedUpdates` would not cause other pending updates to flush too, only the newly mounted root. `flushSync` flushes all pending sync work across all roots. This was to preserve the exact behavior of the Stack implementation. But since it's close enough, let's just use `flushSync`. It's unlikely anyone's app accidentally relies on this subtle difference, and the legacy API is deprecated in 18, anyway.
facebook · Jul 1, 2021 · ed6c091 · ed6c091
1 parent 32eefcb
commit ed6c091
Show file tree

Hide file tree

Showing 12 changed files with 59 additions and 239 deletions.
diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap
@@ -40,7 +40,7 @@ Object {
     6 => 1,
   },
   "passiveEffectDuration": null,
-  "priorityLevel": "Normal",
+  "priorityLevel": "Immediate",
   "timestamp": 16,
   "updaters": Array [
     Object {
@@ -87,7 +87,7 @@ Object {
     4 => 2,
   },
   "passiveEffectDuration": null,
-  "priorityLevel": "Normal",
+  "priorityLevel": "Immediate",
   "timestamp": 15,
   "updaters": Array [
     Object {
@@ -186,7 +186,7 @@ Object {
     6 => 1,
   },
   "passiveEffectDuration": null,
-  "priorityLevel": "Normal",
+  "priorityLevel": "Immediate",
   "timestamp": 12,
   "updaters": Array [
     Object {
@@ -445,7 +445,7 @@ Object {
             ],
           ],
           "passiveEffectDuration": null,
-          "priorityLevel": "Normal",
+          "priorityLevel": "Immediate",
           "timestamp": 12,
           "updaters": Array [
             Object {
@@ -938,7 +938,7 @@ Object {
             ],
           ],
           "passiveEffectDuration": null,
-          "priorityLevel": "Normal",
+          "priorityLevel": "Immediate",
           "timestamp": 11,
           "updaters": Array [
             Object {
@@ -1597,7 +1597,7 @@ Object {
         17 => 1,
       },
       "passiveEffectDuration": null,
-      "priorityLevel": "Normal",
+      "priorityLevel": "Immediate",
       "timestamp": 24,
       "updaters": Array [
         Object {
@@ -1687,7 +1687,7 @@ Object {
       "fiberActualDurations": Map {},
       "fiberSelfDurations": Map {},
       "passiveEffectDuration": 0,
-      "priorityLevel": "Normal",
+      "priorityLevel": "Immediate",
       "timestamp": 34,
       "updaters": Array [
         Object {
@@ -2223,7 +2223,7 @@ Object {
             ],
           ],
           "passiveEffectDuration": null,
-          "priorityLevel": "Normal",
+          "priorityLevel": "Immediate",
           "timestamp": 24,
           "updaters": Array [
             Object {
@@ -2310,7 +2310,7 @@ Object {
           "fiberActualDurations": Array [],
           "fiberSelfDurations": Array [],
           "passiveEffectDuration": 0,
-          "priorityLevel": "Normal",
+          "priorityLevel": "Immediate",
           "timestamp": 34,
           "updaters": Array [
             Object {
@@ -2431,7 +2431,7 @@ Object {
         2 => 0,
       },
       "passiveEffectDuration": null,
-      "priorityLevel": "Normal",
+      "priorityLevel": "Immediate",
       "timestamp": 0,
       "updaters": Array [
         Object {
@@ -2506,7 +2506,7 @@ Object {
     3 => 0,
   },
   "passiveEffectDuration": 0,
-  "priorityLevel": "Normal",
+  "priorityLevel": "Immediate",
   "timestamp": 0,
   "updaters": Array [
     Object {
@@ -2715,7 +2715,7 @@ Object {
             ],
           ],
           "passiveEffectDuration": 0,
-          "priorityLevel": "Normal",
+          "priorityLevel": "Immediate",
           "timestamp": 0,
           "updaters": Array [
             Object {
@@ -3071,7 +3071,7 @@ Object {
     7 => 0,
   },
   "passiveEffectDuration": null,
-  "priorityLevel": "Normal",
+  "priorityLevel": "Immediate",
   "timestamp": 0,
   "updaters": Array [
     Object {
@@ -3515,7 +3515,7 @@ Object {
             ],
           ],
           "passiveEffectDuration": null,
-          "priorityLevel": "Normal",
+          "priorityLevel": "Immediate",
           "timestamp": 0,
           "updaters": Array [
             Object {

diff --git a/packages/react-dom/src/__tests__/ReactMount-test.js b/packages/react-dom/src/__tests__/ReactMount-test.js
@@ -277,7 +277,7 @@ describe('ReactMount', () => {
     expect(calls).toBe(5);
   });
 
-  it('initial mount is sync inside batchedUpdates, but task work is deferred until the end of the batch', () => {
+  it('initial mount of legacy root is sync inside batchedUpdates, as if it were wrapped in flushSync', () => {
     const container1 = document.createElement('div');
     const container2 = document.createElement('div');
 
@@ -302,12 +302,12 @@ describe('ReactMount', () => {
 
       // Initial mount on another root. Should flush immediately.
       ReactDOM.render(<Foo>a</Foo>, container2);
-      // The update did not flush yet.
-      expect(container1.textContent).toEqual('1');
-      // The initial mount flushed, but not the update scheduled in cDM.
-      expect(container2.textContent).toEqual('a');
+      // The earlier update also flushed, since flushSync flushes all pending
+      // sync work across all roots.
+      expect(container1.textContent).toEqual('2');
+      // Layout updates are also flushed synchronously
+      expect(container2.textContent).toEqual('a!');
     });
-    // All updates have flushed.
     expect(container1.textContent).toEqual('2');
     expect(container2.textContent).toEqual('a!');
   });

diff --git a/packages/react-dom/src/client/ReactDOMLegacy.js b/packages/react-dom/src/client/ReactDOMLegacy.js
@@ -29,7 +29,7 @@ import {
   createContainer,
   findHostInstanceWithNoPortals,
   updateContainer,
-  unbatchedUpdates,
+  flushSyncWithoutWarningIfAlreadyRendering,
   getPublicRootInstance,
   findHostInstance,
   findHostInstanceWithWarning,
@@ -174,7 +174,7 @@ function legacyRenderSubtreeIntoContainer(
       };
     }
     // Initial mount should not be batched.
-    unbatchedUpdates(() => {
+    flushSyncWithoutWarningIfAlreadyRendering(() => {
       updateContainer(children, fiberRoot, parentComponent, callback);
     });
   } else {
@@ -357,7 +357,7 @@ export function unmountComponentAtNode(container: Container) {
     }
 
     // Unmount should not be batched.
-    unbatchedUpdates(() => {
+    flushSyncWithoutWarningIfAlreadyRendering(() => {
       legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
         // $FlowFixMe This should probably use `delete container._reactRootContainer`
         container._reactRootContainer = null;

diff --git a/packages/react-noop-renderer/src/ReactNoop.js b/packages/react-noop-renderer/src/ReactNoop.js
@@ -38,7 +38,6 @@ export const {
   flushExpired,
   batchedUpdates,
   deferredUpdates,
-  unbatchedUpdates,
   discreteUpdates,
   idleUpdates,
   flushSync,

diff --git a/packages/react-noop-renderer/src/ReactNoopPersistent.js b/packages/react-noop-renderer/src/ReactNoopPersistent.js
@@ -38,7 +38,6 @@ export const {
   flushExpired,
   batchedUpdates,
   deferredUpdates,
-  unbatchedUpdates,
   discreteUpdates,
   idleUpdates,
   flushDiscreteUpdates,

diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -901,8 +901,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 
     deferredUpdates: NoopRenderer.deferredUpdates,
 
-    unbatchedUpdates: NoopRenderer.unbatchedUpdates,
-
     discreteUpdates: NoopRenderer.discreteUpdates,
 
     idleUpdates<T>(fn: () => T): T {

diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -18,7 +18,6 @@ import {
   createContainer as createContainer_old,
   updateContainer as updateContainer_old,
   batchedUpdates as batchedUpdates_old,
-  unbatchedUpdates as unbatchedUpdates_old,
   deferredUpdates as deferredUpdates_old,
   discreteUpdates as discreteUpdates_old,
   flushControlled as flushControlled_old,
@@ -56,7 +55,6 @@ import {
   createContainer as createContainer_new,
   updateContainer as updateContainer_new,
   batchedUpdates as batchedUpdates_new,
-  unbatchedUpdates as unbatchedUpdates_new,
   deferredUpdates as deferredUpdates_new,
   discreteUpdates as discreteUpdates_new,
   flushControlled as flushControlled_new,
@@ -99,9 +97,6 @@ export const updateContainer = enableNewReconciler
 export const batchedUpdates = enableNewReconciler
   ? batchedUpdates_new
   : batchedUpdates_old;
-export const unbatchedUpdates = enableNewReconciler
-  ? unbatchedUpdates_new
-  : unbatchedUpdates_old;
 export const deferredUpdates = enableNewReconciler
   ? deferredUpdates_new
   : deferredUpdates_old;

diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js
@@ -52,7 +52,6 @@ import {
   scheduleUpdateOnFiber,
   flushRoot,
   batchedUpdates,
-  unbatchedUpdates,
   flushSync,
   flushControlled,
   deferredUpdates,
@@ -327,7 +326,6 @@ export function updateContainer(
 
 export {
   batchedUpdates,
-  unbatchedUpdates,
   deferredUpdates,
   discreteUpdates,
   flushControlled,

diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js
@@ -52,7 +52,6 @@ import {
   scheduleUpdateOnFiber,
   flushRoot,
   batchedUpdates,
-  unbatchedUpdates,
   flushSync,
   flushControlled,
   deferredUpdates,
@@ -327,7 +326,6 @@ export function updateContainer(
 
 export {
   batchedUpdates,
-  unbatchedUpdates,
   deferredUpdates,
   discreteUpdates,
   flushControlled,

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -246,12 +246,11 @@ const {
 
 type ExecutionContext = number;
 
-export const NoContext = /*             */ 0b00000;
-const BatchedContext = /*               */ 0b00001;
-const LegacyUnbatchedContext = /*       */ 0b00010;
-const RenderContext = /*                */ 0b00100;
-const CommitContext = /*                */ 0b01000;
-export const RetryAfterError = /*       */ 0b10000;
+export const NoContext = /*             */ 0b0000;
+const BatchedContext = /*               */ 0b0001;
+const RenderContext = /*                */ 0b0010;
+const CommitContext = /*                */ 0b0100;
+export const RetryAfterError = /*       */ 0b1000;
 
 type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
 const RootIncomplete = 0;
@@ -515,35 +514,19 @@ export function scheduleUpdateOnFiber(
     }
   }
 
-  if (lane === SyncLane) {
-    if (
-      // Check if we're inside unbatchedUpdates
-      (executionContext & LegacyUnbatchedContext) !== NoContext &&
-      // Check if we're not already rendering
-      (executionContext & (RenderContext | CommitContext)) === NoContext
-    ) {
-      // This is a legacy edge case. The initial mount of a ReactDOM.render-ed
-      // root inside of batchedUpdates should be synchronous, but layout updates
-      // should be deferred until the end of the batch.
-      performSyncWorkOnRoot(root);
-    } else {
-      ensureRootIsScheduled(root, eventTime);
-      if (
-        executionContext === NoContext &&
-        (fiber.mode & ConcurrentMode) === NoMode
-      ) {
-        // Flush the synchronous work now, unless we're already working or inside
-        // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
-        // scheduleCallbackForFiber to preserve the ability to schedule a callback
-        // without immediately flushing it. We only do this for user-initiated
-        // updates, to preserve historical behavior of legacy mode.
-        resetRenderTimer();
-        flushSyncCallbacksOnlyInLegacyMode();
-      }
-    }
-  } else {
-    // Schedule other updates after in case the callback is sync.
-    ensureRootIsScheduled(root, eventTime);
+  ensureRootIsScheduled(root, eventTime);
+  if (
+    lane === SyncLane &&
+    executionContext === NoContext &&
+    (fiber.mode & ConcurrentMode) === NoMode
+  ) {
+    // Flush the synchronous work now, unless we're already working or inside
+    // a batch. This is intentionally inside scheduleUpdateOnFiber instead of
+    // scheduleCallbackForFiber to preserve the ability to schedule a callback
+    // without immediately flushing it. We only do this for user-initiated
+    // updates, to preserve historical behavior of legacy mode.
+    resetRenderTimer();
+    flushSyncCallbacksOnlyInLegacyMode();
   }
 
   return root;
@@ -1095,25 +1078,6 @@ export function discreteUpdates<A, B, C, D, R>(
   }
 }
 
-export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
-  const prevExecutionContext = executionContext;
-  executionContext &= ~BatchedContext;
-  executionContext |= LegacyUnbatchedContext;
-  try {
-    return fn(a);
-  } finally {
-    executionContext = prevExecutionContext;
-    // If there were legacy sync updates, flush them at the end of the outer
-    // most batchedUpdates-like method.
-    if (executionContext === NoContext) {
-      resetRenderTimer();
-      // TODO: I think this call is redundant, because we flush inside
-      // scheduleUpdateOnFiber when LegacyUnbatchedContext is set.
-      flushSyncCallbacksOnlyInLegacyMode();
-    }
-  }
-}
-
 export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
   fn: A => R,
   a: A,
@@ -1954,24 +1918,6 @@ function commitRootImpl(root, renderPriorityLevel) {
     throw error;
   }
 
-  if ((executionContext & LegacyUnbatchedContext) !== NoContext) {
-    if (__DEV__) {
-      if (enableDebugTracing) {
-        logCommitStopped();
-      }
-    }
-
-    if (enableSchedulingProfiler) {
-      markCommitStopped();
-    }
-
-    // This is a legacy edge case. We just committed the initial mount of
-    // a ReactDOM.render-ed root inside of batchedUpdates. The commit fired
-    // synchronously, but layout updates should be deferred until the end
-    // of the batch.
-    return null;
-  }
-
   // If the passive effects are the result of a discrete render, flush them
   // synchronously at the end of the current task so that the result is
   // immediately observable. Otherwise, we assume that they are not