Implements flushSync in react-dom without relying on the reconciler. This will only be available in builds that no longer support legacy mode because the reconciler flushSync has special logic for legacy mode which is not necessary for concurrent roots.

gnoff · gnoff · commit 7e746d53e5ad · 2024-04-05T15:27:03.000-07:00
This is one option for how we might recover existing flushSync priorities with an external flushSync implementation.

To reduce hidden class checks we need to read the batch config once which requires inlining the requestCurrentTransition implementation.

Moves the legacy implementation of flushSync to the fb entrypoint

The cost of the separate file is not really warranted. Will use feature flag to scope build specific implementations

Moved error to fiber config. The reconciler implementation should be DCE'd in builds that still support legacy mode

Exposes an updateContainerSync implementation so we can avoid depending on flushSyncFromReconciler in hot reloading

Removes flushSyncFromReconciler from ReactDOMRoot

Removes flushSyncFromReconciler from ReactDOMUpdateBatching

Removes flushSyncFromReconciler use from ReactFiberReconciler

Removes flushSyncFromReconciler from ReactART

Rather than use event priority or lane type semantics for the batch config it now uses a boolean. There really is only a binary of sync or not sync so we don't need to express this concept as something overly specific to Fiber.

simplifies the during render case to in dev to avoid a bit of extra work. There is no harm in returning true in prod even if there is nothing to react to it.
diff --git a/packages/react-art/src/ReactART.js b/packages/react-art/src/ReactART.js
@@ -10,9 +10,10 @@ import ReactVersion from 'shared/ReactVersion';
 import {LegacyRoot, ConcurrentRoot} from 'react-reconciler/src/ReactRootTags';
 import {
   createContainer,
-  updateContainer,
+  updateContainerSync,
   injectIntoDevTools,
-  flushSync,
+  flushSyncWork,
+  flushPassiveEffects,
 } from 'react-reconciler/src/ReactFiberReconciler';
 import Transform from 'art/core/transform';
 import Mode from 'art/modes/current';
@@ -78,9 +79,8 @@ class Surface extends React.Component {
     );
     // We synchronously flush updates coming from above so that they commit together
     // and so that refs resolve before the parent life cycles.
-    flushSync(() => {
-      updateContainer(this.props.children, this._mountNode, this);
-    });
+    updateContainerSync(this.props.children, this._mountNode, this);
+    flushSyncWork();
   }
 
   componentDidUpdate(prevProps, prevState) {
@@ -92,9 +92,8 @@ class Surface extends React.Component {
 
     // We synchronously flush updates coming from above so that they commit together
     // and so that refs resolve before the parent life cycles.
-    flushSync(() => {
-      updateContainer(this.props.children, this._mountNode, this);
-    });
+    updateContainerSync(this.props.children, this._mountNode, this);
+    flushSyncWork();
 
     if (this._surface.render) {
       this._surface.render();
@@ -104,9 +103,8 @@ class Surface extends React.Component {
   componentWillUnmount() {
     // We synchronously flush updates coming from above so that they commit together
     // and so that refs resolve before the parent life cycles.
-    flushSync(() => {
-      updateContainer(null, this._mountNode, this);
-    });
+    updateContainerSync(null, this._mountNode, this);
+    flushSyncWork();
   }
 
   render() {
diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
@@ -90,6 +90,7 @@ import {
   enableScopeAPI,
   enableTrustedTypesIntegration,
   enableAsyncActions,
+  disableLegacyMode,
 } from 'shared/ReactFeatureFlags';
 import {
   HostComponent,
@@ -100,6 +101,7 @@ import {
 import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';
 import {validateLinkPropsForStyleResource} from '../shared/ReactDOMResourceValidation';
 import escapeSelectorAttributeValueInsideDoubleQuotes from './escapeSelectorAttributeValueInsideDoubleQuotes';
+import {flushSyncWork as flushSyncWorkOnAllRoots} from 'react-reconciler/src/ReactFiberWorkLoop';
 
 import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals';
 const ReactDOMCurrentDispatcher =
@@ -1924,6 +1926,9 @@ function getDocumentFromRoot(root: HoistableRoot): Document {
 
 const previousDispatcher = ReactDOMCurrentDispatcher.current;
 ReactDOMCurrentDispatcher.current = {
+  flushSyncWork: disableLegacyMode
+    ? flushSyncWork
+    : previousDispatcher.flushSyncWork,
   prefetchDNS,
   preconnect,
   preload,
@@ -1933,6 +1938,20 @@ ReactDOMCurrentDispatcher.current = {
   preinitModuleScript,
 };
 
+function flushSyncWork() {
+  if (disableLegacyMode) {
+    const previousWasRendering = previousDispatcher.flushSyncWork();
+    const wasRendering = flushSyncWorkOnAllRoots();
+    // Since multiple dispatchers can flush sync work during a single flushSync call
+    // we need to return true if any of them were rendering.
+    return previousWasRendering || wasRendering;
+  } else {
+    throw new Error(
+      'flushSyncWork should not be called from builds that support legacy mode. This is a bug in React.',
+    );
+  }
+}
+
 // We expect this to get inlined. It is a function mostly to communicate the special nature of
 // how we resolve the HoistableRoot for ReactDOM.pre*() methods. Because we support calling
 // these methods outside of render there is no way to know which Document or ShadowRoot is 'scoped'
diff --git a/packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js b/packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js
@@ -13,7 +13,7 @@ import {
 import {
   batchedUpdates as batchedUpdatesImpl,
   discreteUpdates as discreteUpdatesImpl,
-  flushSync as flushSyncImpl,
+  flushSyncWork,
 } from 'react-reconciler/src/ReactFiberReconciler';
 
 // Used as a way to call batchedUpdates when we don't have a reference to
@@ -36,7 +36,9 @@ function finishEventHandler() {
     // bails out of the update without touching the DOM.
     // TODO: Restore state in the microtask, after the discrete updates flush,
     // instead of early flushing them here.
-    flushSyncImpl();
+    // @TODO Should move to flushSyncWork once legacy mode is removed but since this flushSync
+    // flushes passive effects we can't do this yet.
+    flushSyncWork();
     restoreStateIfNeeded();
   }
 }
diff --git a/packages/react-dom-bindings/src/server/ReactDOMFlightServerHostDispatcher.js b/packages/react-dom-bindings/src/server/ReactDOMFlightServerHostDispatcher.js
@@ -28,6 +28,7 @@ const ReactDOMCurrentDispatcher =
 
 const previousDispatcher = ReactDOMCurrentDispatcher.current;
 ReactDOMCurrentDispatcher.current = {
+  flushSyncWork: previousDispatcher.flushSyncWork,
   prefetchDNS,
   preconnect,
   preload,
diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -88,6 +88,7 @@ const ReactDOMCurrentDispatcher =
 
 const previousDispatcher = ReactDOMCurrentDispatcher.current;
 ReactDOMCurrentDispatcher.current = {
+  flushSyncWork: previousDispatcher.flushSyncWork,
   prefetchDNS,
   preconnect,
   preload,
diff --git a/packages/react-dom/src/ReactDOMSharedInternals.js b/packages/react-dom/src/ReactDOMSharedInternals.js
@@ -29,6 +29,7 @@ type InternalsType = {
 function noop() {}
 
 const DefaultDispatcher: HostDispatcher = {
+  flushSyncWork: noop,
   prefetchDNS: noop,
   preconnect: noop,
   preload: noop,
diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js
@@ -14,16 +14,18 @@ import type {
   CreateRootOptions,
 } from './ReactDOMRoot';
 
+import {disableLegacyMode} from 'shared/ReactFeatureFlags';
 import {
   createRoot as createRootImpl,
   hydrateRoot as hydrateRootImpl,
   isValidContainer,
 } from './ReactDOMRoot';
 import {createEventHandle} from 'react-dom-bindings/src/client/ReactDOMEventHandle';
 import {runWithPriority} from 'react-dom-bindings/src/client/ReactDOMUpdatePriority';
+import {flushSync as flushSyncIsomorphic} from '../shared/ReactDOMFlushSync';
 
 import {
-  flushSync as flushSyncWithoutWarningIfAlreadyRendering,
+  flushSyncFromReconciler as flushSyncWithoutWarningIfAlreadyRendering,
   isAlreadyRendering,
   injectIntoDevTools,
   findHostInstance,
@@ -123,11 +125,11 @@ function hydrateRoot(
 
 // Overload the definition to the two valid signatures.
 // Warning, this opts-out of checking the function body.
-declare function flushSync<R>(fn: () => R): R;
+declare function flushSyncFromReconciler<R>(fn: () => R): R;
 // eslint-disable-next-line no-redeclare
-declare function flushSync(): void;
+declare function flushSyncFromReconciler(): void;
 // eslint-disable-next-line no-redeclare
-function flushSync<R>(fn: (() => R) | void): R | void {
+function flushSyncFromReconciler<R>(fn: (() => R) | void): R | void {
   if (__DEV__) {
     if (isAlreadyRendering()) {
       console.error(
@@ -140,6 +142,10 @@ function flushSync<R>(fn: (() => R) | void): R | void {
   return flushSyncWithoutWarningIfAlreadyRendering(fn);
 }
 
+const flushSync: typeof flushSyncIsomorphic = disableLegacyMode
+  ? flushSyncIsomorphic
+  : flushSyncFromReconciler;
+
 function findDOMNode(
   componentOrElement: React$Component<any, any>,
 ): null | Element | Text {
diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js
@@ -93,7 +93,8 @@ import {
   createContainer,
   createHydrationContainer,
   updateContainer,
-  flushSync,
+  updateContainerSync,
+  flushSyncWork,
   isAlreadyRendering,
   defaultOnUncaughtError,
   defaultOnCaughtError,
@@ -161,9 +162,8 @@ ReactDOMHydrationRoot.prototype.unmount = ReactDOMRoot.prototype.unmount =
           );
         }
       }
-      flushSync(() => {
-        updateContainer(null, root, null, null);
-      });
+      updateContainerSync(null, root, null, null);
+      flushSyncWork();
       unmarkContainerAsRoot(container);
     }
   };
diff --git a/packages/react-dom/src/client/ReactDOMRootFB.js b/packages/react-dom/src/client/ReactDOMRootFB.js
@@ -49,7 +49,7 @@ import {
   createHydrationContainer,
   findHostInstanceWithNoPortals,
   updateContainer,
-  flushSync,
+  flushSyncFromReconciler,
   getPublicRootInstance,
   findHostInstance,
   findHostInstanceWithWarning,
@@ -247,7 +247,7 @@ function legacyCreateRootFromDOMContainer(
     // $FlowFixMe[incompatible-call]
     listenToAllSupportedEvents(rootContainerElement);
 
-    flushSync();
+    flushSyncFromReconciler();
     return root;
   } else {
     // First clear any existing content.
@@ -282,7 +282,7 @@ function legacyCreateRootFromDOMContainer(
     listenToAllSupportedEvents(rootContainerElement);
 
     // Initial mount should not be batched.
-    flushSync(() => {
+    flushSyncFromReconciler(() => {
       updateContainer(initialChildren, root, parentComponent, callback);
     });
 
@@ -497,7 +497,7 @@ export function unmountComponentAtNode(container: Container): boolean {
     }
 
     // Unmount should not be batched.
-    flushSync(() => {
+    flushSyncFromReconciler(() => {
       legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
         // $FlowFixMe[incompatible-type] This should probably use `delete container._reactRootContainer`
         container._reactRootContainer = null;
diff --git a/packages/react-dom/src/shared/ReactDOMFlushSync.js b/packages/react-dom/src/shared/ReactDOMFlushSync.js
@@ -0,0 +1,66 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+import type {BatchConfig} from 'react/src/ReactCurrentBatchConfig';
+
+import {disableLegacyMode} from 'shared/ReactFeatureFlags';
+import {DiscreteEventPriority} from 'react-reconciler/src/ReactEventPriorities';
+
+import ReactSharedInternals from 'shared/ReactSharedInternals';
+const ReactCurrentBatchConfig: BatchConfig =
+  ReactSharedInternals.ReactCurrentBatchConfig;
+
+import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals';
+const ReactDOMCurrentDispatcher =
+  ReactDOMSharedInternals.ReactDOMCurrentDispatcher;
+const ReactDOMCurrentUpdatePriority =
+  ReactDOMSharedInternals.ReactDOMCurrentUpdatePriority;
+
+declare function flushSyncImpl<R>(fn: () => R): R;
+declare function flushSyncImpl(void): void;
+function flushSyncImpl<R>(fn: (() => R) | void): R | void {
+  const previousTransition = ReactCurrentBatchConfig.transition;
+  const previousUpdatePriority = ReactDOMCurrentUpdatePriority.current;
+
+  try {
+    ReactCurrentBatchConfig.transition = null;
+    ReactDOMCurrentUpdatePriority.current = DiscreteEventPriority;
+    if (fn) {
+      return fn();
+    } else {
+      return undefined;
+    }
+  } finally {
+    ReactCurrentBatchConfig.transition = previousTransition;
+    ReactDOMCurrentUpdatePriority.current = previousUpdatePriority;
+    const wasInRender = ReactDOMCurrentDispatcher.current.flushSyncWork();
+    if (__DEV__) {
+      if (wasInRender) {
+        console.error(
+          'flushSync was called from inside a lifecycle method. React cannot ' +
+            'flush when React is already rendering. Consider moving this call to ' +
+            'a scheduler task or micro task.',
+        );
+      }
+    }
+  }
+}
+
+declare function flushSyncErrorInBuildsThatSupportLegacyMode<R>(fn: () => R): R;
+declare function flushSyncErrorInBuildsThatSupportLegacyMode(void): void;
+function flushSyncErrorInBuildsThatSupportLegacyMode() {
+  // eslint-disable-next-line react-internal/prod-error-codes
+  throw new Error(
+    'Expected this build of React to not support legacy mode but it does. This is a bug in React.',
+  );
+}
+
+export const flushSync: typeof flushSyncImpl = disableLegacyMode
+  ? flushSyncImpl
+  : flushSyncErrorInBuildsThatSupportLegacyMode;
diff --git a/packages/react-dom/src/shared/ReactDOMTypes.js b/packages/react-dom/src/shared/ReactDOMTypes.js
@@ -82,6 +82,7 @@ export type PreinitModuleScriptOptions = {
 };
 
 export type HostDispatcher = {
+  flushSyncWork: () => boolean | void,
   prefetchDNS: (href: string) => void,
   preconnect: (href: string, crossOrigin?: ?CrossOriginEnum) => void,
   preload: (href: string, as: string, options?: ?PreloadImplOptions) => void,
diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -29,13 +29,17 @@ import isArray from 'shared/isArray';
 import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
 import {
   NoEventPriority,
+  DiscreteEventPriority,
   DefaultEventPriority,
   IdleEventPriority,
   ConcurrentRoot,
   LegacyRoot,
 } from 'react-reconciler/constants';
 import {enableRefAsProp, disableLegacyMode} from 'shared/ReactFeatureFlags';
 
+import ReactSharedInternals from 'shared/ReactSharedInternals';
+const ReactCurrentBatchConfig = ReactSharedInternals.ReactCurrentBatchConfig;
+
 type Container = {
   rootID: string,
   children: Array<Instance | TextInstance>,
@@ -931,7 +935,25 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
         );
       }
     }
-    return NoopRenderer.flushSync(fn);
+    if (disableLegacyMode) {
+      const previousTransition = ReactCurrentBatchConfig.transition;
+      const preivousEventPriority = currentEventPriority;
+      try {
+        ReactCurrentBatchConfig.transition = null;
+        currentEventPriority = DiscreteEventPriority;
+        if (fn) {
+          return fn();
+        } else {
+          return undefined;
+        }
+      } finally {
+        ReactCurrentBatchConfig.transition = previousTransition;
+        currentEventPriority = preivousEventPriority;
+        NoopRenderer.flushSyncWork();
+      }
+    } else {
+      return NoopRenderer.flushSyncFromReconciler(fn);
+    }
   }
 
   function onRecoverableError(error) {
@@ -1069,6 +1091,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
         getChildrenAsJSX() {
           return getChildrenAsJSX(container);
         },
+        legacy: true,
       };
     },
 
diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.js b/packages/react-reconciler/src/ReactFiberHotReloading.js
diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js
diff --git a/packages/react/src/ReactCurrentBatchConfig.js b/packages/react/src/ReactCurrentBatchConfig.js
diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
