Skip to content

Commit

Permalink
[DOM] move flushSync out of the reconciler (#28500)
Browse files Browse the repository at this point in the history
This PR moves `flushSync` out of the reconciler. there is still an
internal implementation that is used when these semantics are needed for
React methods such as `unmount` on roots.

This new isomorphic `flushSync` is only used in builds that no longer
support legacy mode.

Additionally all the internal uses of flushSync in the reconciler have
been replaced with more direct methods. There is a new
`updateContainerSync` method which updates a container but forces it to
the Sync lane and flushes passive effects if necessary. This combined
with flushSyncWork can be used to replace flushSync for all instances of
internal usage.

We still maintain the original flushSync implementation as
`flushSyncFromReconciler` because it will be used as the flushSync
implementation for FB builds. This is because it has special legacy mode
handling that the new isomorphic implementation does not need to
consider. It will be removed from production OSS builds by closure
though
  • Loading branch information
gnoff authored and rickhanlonii committed Apr 11, 2024
1 parent 24ceaa9 commit f35b382
Show file tree
Hide file tree
Showing 18 changed files with 248 additions and 84 deletions.
19 changes: 8 additions & 11 deletions packages/react-art/src/ReactART.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import ReactVersion from 'shared/ReactVersion';
import {LegacyRoot, ConcurrentRoot} from 'react-reconciler/src/ReactRootTags';
import {
createContainer,
updateContainer,
updateContainerSync,
injectIntoDevTools,
flushSync,
flushSyncWork,
} from 'react-reconciler/src/ReactFiberReconciler';
import Transform from 'art/core/transform';
import Mode from 'art/modes/current';
Expand Down Expand Up @@ -78,9 +78,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) {
Expand All @@ -92,9 +91,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();
Expand All @@ -104,9 +102,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() {
Expand Down
19 changes: 19 additions & 0 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ import {
enableScopeAPI,
enableTrustedTypesIntegration,
enableAsyncActions,
disableLegacyMode,
} from 'shared/ReactFeatureFlags';
import {
HostComponent,
Expand All @@ -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 =
Expand Down Expand Up @@ -1924,6 +1926,9 @@ function getDocumentFromRoot(root: HoistableRoot): Document {

const previousDispatcher = ReactDOMCurrentDispatcher.current;
ReactDOMCurrentDispatcher.current = {
flushSyncWork: disableLegacyMode
? flushSyncWork
: previousDispatcher.flushSyncWork,
prefetchDNS,
preconnect,
preload,
Expand All @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const ReactDOMCurrentDispatcher =

const previousDispatcher = ReactDOMCurrentDispatcher.current;
ReactDOMCurrentDispatcher.current = {
flushSyncWork: previousDispatcher.flushSyncWork,
prefetchDNS,
preconnect,
preload,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ const ReactDOMCurrentDispatcher =

const previousDispatcher = ReactDOMCurrentDispatcher.current;
ReactDOMCurrentDispatcher.current = {
flushSyncWork: previousDispatcher.flushSyncWork,
prefetchDNS,
preconnect,
preload,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/ReactDOMSharedInternals.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type InternalsType = {
function noop() {}

const DefaultDispatcher: HostDispatcher = {
flushSyncWork: noop,
prefetchDNS: noop,
preconnect: noop,
preload: noop,
Expand Down
14 changes: 10 additions & 4 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ import {
createContainer,
createHydrationContainer,
updateContainer,
flushSync,
updateContainerSync,
flushSyncWork,
isAlreadyRendering,
defaultOnUncaughtError,
defaultOnCaughtError,
Expand Down Expand Up @@ -161,9 +162,8 @@ ReactDOMHydrationRoot.prototype.unmount = ReactDOMRoot.prototype.unmount =
);
}
}
flushSync(() => {
updateContainer(null, root, null, null);
});
updateContainerSync(null, root, null, null);
flushSyncWork();
unmarkContainerAsRoot(container);
}
};
Expand Down
27 changes: 12 additions & 15 deletions packages/react-dom/src/client/ReactDOMRootFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ import {
createHydrationContainer,
findHostInstanceWithNoPortals,
updateContainer,
flushSync,
updateContainerSync,
flushSyncWork,
getPublicRootInstance,
findHostInstance,
findHostInstanceWithWarning,
Expand Down Expand Up @@ -247,7 +248,7 @@ function legacyCreateRootFromDOMContainer(
// $FlowFixMe[incompatible-call]
listenToAllSupportedEvents(rootContainerElement);

flushSync();
flushSyncWork();
return root;
} else {
// First clear any existing content.
Expand Down Expand Up @@ -282,9 +283,8 @@ function legacyCreateRootFromDOMContainer(
listenToAllSupportedEvents(rootContainerElement);

// Initial mount should not be batched.
flushSync(() => {
updateContainer(initialChildren, root, parentComponent, callback);
});
updateContainerSync(initialChildren, root, parentComponent, callback);
flushSyncWork();

return root;
}
Expand Down Expand Up @@ -485,6 +485,8 @@ export function unmountComponentAtNode(container: Container): boolean {
}

if (container._reactRootContainer) {
const root = container._reactRootContainer;

if (__DEV__) {
const rootEl = getReactRootElementInContainer(container);
const renderedByDifferentReact = rootEl && !getInstanceFromNode(rootEl);
Expand All @@ -496,16 +498,11 @@ export function unmountComponentAtNode(container: Container): boolean {
}
}

// Unmount should not be batched.
flushSync(() => {
legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
// $FlowFixMe[incompatible-type] This should probably use `delete container._reactRootContainer`
container._reactRootContainer = null;
unmarkContainerAsRoot(container);
});
});
// If you call unmountComponentAtNode twice in quick succession, you'll
// get `true` twice. That's probably fine?
updateContainerSync(null, root, null, null);
flushSyncWork();
// $FlowFixMe[incompatible-type] This should probably use `delete container._reactRootContainer`
container._reactRootContainer = null;
unmarkContainerAsRoot(container);
return true;
} else {
if (__DEV__) {
Expand Down
67 changes: 67 additions & 0 deletions packages/react-dom/src/shared/ReactDOMFlushSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* 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;

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 =
ReactDOMSharedInternals.up; /* ReactDOMCurrentUpdatePriority */

try {
ReactCurrentBatchConfig.transition = null;
ReactDOMSharedInternals.up /* ReactDOMCurrentUpdatePriority */ =
DiscreteEventPriority;
if (fn) {
return fn();
} else {
return undefined;
}
} finally {
ReactCurrentBatchConfig.transition = previousTransition;
ReactDOMSharedInternals.up /* ReactDOMCurrentUpdatePriority */ =
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;
1 change: 1 addition & 0 deletions packages/react-dom/src/shared/ReactDOMTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
25 changes: 24 additions & 1 deletion packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import isArray from 'shared/isArray';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {
NoEventPriority,
DiscreteEventPriority,
DefaultEventPriority,
IdleEventPriority,
ConcurrentRoot,
Expand All @@ -40,6 +41,9 @@ import {
disableStringRefs,
} from 'shared/ReactFeatureFlags';

import ReactSharedInternals from 'shared/ReactSharedInternals';
const ReactCurrentBatchConfig = ReactSharedInternals.ReactCurrentBatchConfig;

type Container = {
rootID: string,
children: Array<Instance | TextInstance>,
Expand Down Expand Up @@ -943,7 +947,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) {
Expand Down Expand Up @@ -1081,6 +1103,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
getChildrenAsJSX() {
return getChildrenAsJSX(container);
},
legacy: true,
};
},

Expand Down
Loading

0 comments on commit f35b382

Please sign in to comment.