From a57f40d839bf055f910cc20d7e501a10df92db98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 14 Mar 2023 21:40:43 -0400 Subject: [PATCH] Undo dependency injection of batching (#26389) There's currently a giant cycle between the event system, through react-dom-bindings, reconciler and then react-dom. We resolve this cycle using dependency injection. However, this all ends up in the same bundle. It can be reordered to resolve the cycles. If we avoid side-effects and avoid reading from module exports during initialization, this should be resolvable in a more optimal way by the compiler. --- .../src/events/ReactDOMControlledComponent.js | 22 +++----- .../src/events/ReactDOMEventListener.js | 2 +- .../src/events/ReactDOMEventReplaying.js | 53 ++++--------------- .../src/events/ReactDOMUpdateBatching.js | 25 +++------ packages/react-dom/src/client/ReactDOM.js | 35 +----------- 5 files changed, 25 insertions(+), 112 deletions(-) diff --git a/packages/react-dom-bindings/src/events/ReactDOMControlledComponent.js b/packages/react-dom-bindings/src/events/ReactDOMControlledComponent.js index d1cf123ef6961..ced86d3733993 100644 --- a/packages/react-dom-bindings/src/events/ReactDOMControlledComponent.js +++ b/packages/react-dom-bindings/src/events/ReactDOMControlledComponent.js @@ -12,9 +12,10 @@ import { getFiberCurrentPropsFromNode, } from '../client/ReactDOMComponentTree'; +import {restoreControlledState} from 'react-dom-bindings/src/client/ReactDOMComponent'; + // Use to restore controlled state after a change event has fired. -let restoreImpl = null; let restoreTarget = null; let restoreQueue = null; @@ -27,27 +28,18 @@ function restoreStateOfTarget(target: Node) { return; } - if (typeof restoreImpl !== 'function') { - throw new Error( - 'setRestoreImplementation() needs to be called to handle a target for controlled ' + - 'events. This error is likely caused by a bug in React. Please file an issue.', - ); - } - const stateNode = internalInstance.stateNode; // Guard against Fiber being unmounted. if (stateNode) { const props = getFiberCurrentPropsFromNode(stateNode); - restoreImpl(internalInstance.stateNode, internalInstance.type, props); + restoreControlledState( + internalInstance.stateNode, + internalInstance.type, + props, + ); } } -export function setRestoreImplementation( - impl: (domElement: Element, tag: string, props: Object) => void, -): void { - restoreImpl = impl; -} - export function enqueueStateRestore(target: Node): void { if (restoreTarget) { if (restoreQueue) { diff --git a/packages/react-dom-bindings/src/events/ReactDOMEventListener.js b/packages/react-dom-bindings/src/events/ReactDOMEventListener.js index 487420be00917..d5dcb5593d1d4 100644 --- a/packages/react-dom-bindings/src/events/ReactDOMEventListener.js +++ b/packages/react-dom-bindings/src/events/ReactDOMEventListener.js @@ -19,8 +19,8 @@ import { hasQueuedDiscreteEvents, clearIfContinuousEvent, queueIfContinuousEvent, - attemptSynchronousHydration, } from './ReactDOMEventReplaying'; +import {attemptSynchronousHydration} from 'react-reconciler/src/ReactFiberReconciler'; import { getNearestMountedFiber, getContainerFromFiber, diff --git a/packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js b/packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js index 8b61d7607024d..56cf0ba5f45e1 100644 --- a/packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js @@ -38,49 +38,16 @@ import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags'; import {isHigherEventPriority} from 'react-reconciler/src/ReactEventPriorities'; import {isRootDehydrated} from 'react-reconciler/src/ReactFiberShellHydration'; -let _attemptSynchronousHydration: (fiber: Object) => void; - -export function setAttemptSynchronousHydration(fn: (fiber: Object) => void) { - _attemptSynchronousHydration = fn; -} - -export function attemptSynchronousHydration(fiber: Object) { - _attemptSynchronousHydration(fiber); -} - -let attemptDiscreteHydration: (fiber: Object) => void; - -export function setAttemptDiscreteHydration(fn: (fiber: Object) => void) { - attemptDiscreteHydration = fn; -} - -let attemptContinuousHydration: (fiber: Object) => void; - -export function setAttemptContinuousHydration(fn: (fiber: Object) => void) { - attemptContinuousHydration = fn; -} - -let attemptHydrationAtCurrentPriority: (fiber: Object) => void; - -export function setAttemptHydrationAtCurrentPriority( - fn: (fiber: Object) => void, -) { - attemptHydrationAtCurrentPriority = fn; -} - -let getCurrentUpdatePriority: () => EventPriority; - -export function setGetCurrentUpdatePriority(fn: () => EventPriority) { - getCurrentUpdatePriority = fn; -} - -let attemptHydrationAtPriority: (priority: EventPriority, fn: () => T) => T; - -export function setAttemptHydrationAtPriority( - fn: (priority: EventPriority, fn: () => T) => T, -) { - attemptHydrationAtPriority = fn; -} +import { + attemptSynchronousHydration, + attemptDiscreteHydration, + attemptContinuousHydration, + attemptHydrationAtCurrentPriority, +} from 'react-reconciler/src/ReactFiberReconciler'; +import { + runWithPriority as attemptHydrationAtPriority, + getCurrentUpdatePriority, +} from 'react-reconciler/src/ReactEventPriorities'; // TODO: Upgrade this definition once we're on a newer version of Flow that // has this definition built-in. diff --git a/packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js b/packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js index f5755865585b9..0e80a3e7504b6 100644 --- a/packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js +++ b/packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js @@ -10,21 +10,18 @@ import { restoreStateIfNeeded, } from './ReactDOMControlledComponent'; +import { + batchedUpdates as batchedUpdatesImpl, + discreteUpdates as discreteUpdatesImpl, + flushSync as flushSyncImpl, +} from 'react-reconciler/src/ReactFiberReconciler'; + // Used as a way to call batchedUpdates when we don't have a reference to // the renderer. Such as when we're dispatching events or if third party // libraries need to call batchedUpdates. Eventually, this API will go away when // everything is batched by default. We'll then have a similar API to opt-out of // scheduled work and instead do synchronous work. -// Defaults -let batchedUpdatesImpl = function (fn, bookkeeping) { - return fn(bookkeeping); -}; -let discreteUpdatesImpl = function (fn, a, b, c, d) { - return fn(a, b, c, d); -}; -let flushSyncImpl = function () {}; - let isInsideEventHandler = false; function finishEventHandler() { @@ -63,13 +60,3 @@ export function batchedUpdates(fn, a, b) { export function discreteUpdates(fn, a, b, c, d) { return discreteUpdatesImpl(fn, a, b, c, d); } - -export function setBatchingImplementation( - _batchedUpdatesImpl, - _discreteUpdatesImpl, - _flushSyncImpl, -) { - batchedUpdatesImpl = _batchedUpdatesImpl; - discreteUpdatesImpl = _discreteUpdatesImpl; - flushSyncImpl = _flushSyncImpl; -} diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index aa7526e15d192..42fcd1648f6ef 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -34,20 +34,12 @@ import {createEventHandle} from 'react-dom-bindings/src/client/ReactDOMEventHand import { batchedUpdates, - discreteUpdates, flushSync as flushSyncWithoutWarningIfAlreadyRendering, isAlreadyRendering, flushControlled, injectIntoDevTools, - attemptSynchronousHydration, - attemptDiscreteHydration, - attemptContinuousHydration, - attemptHydrationAtCurrentPriority, } from 'react-reconciler/src/ReactFiberReconciler'; -import { - runWithPriority, - getCurrentUpdatePriority, -} from 'react-reconciler/src/ReactEventPriorities'; +import {runWithPriority} from 'react-reconciler/src/ReactEventPriorities'; import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; import ReactVersion from 'shared/ReactVersion'; @@ -58,18 +50,7 @@ import { getNodeFromInstance, getFiberCurrentPropsFromNode, } from 'react-dom-bindings/src/client/ReactDOMComponentTree'; -import {restoreControlledState} from 'react-dom-bindings/src/client/ReactDOMComponent'; -import { - setAttemptSynchronousHydration, - setAttemptDiscreteHydration, - setAttemptContinuousHydration, - setAttemptHydrationAtCurrentPriority, - setGetCurrentUpdatePriority, - setAttemptHydrationAtPriority, -} from 'react-dom-bindings/src/events/ReactDOMEventReplaying'; -import {setBatchingImplementation} from 'react-dom-bindings/src/events/ReactDOMUpdateBatching'; import { - setRestoreImplementation, enqueueStateRestore, restoreStateIfNeeded, } from 'react-dom-bindings/src/events/ReactDOMControlledComponent'; @@ -82,13 +63,6 @@ export { preinit, } from 'react-dom-bindings/src/shared/ReactDOMFloat'; -setAttemptSynchronousHydration(attemptSynchronousHydration); -setAttemptDiscreteHydration(attemptDiscreteHydration); -setAttemptContinuousHydration(attemptContinuousHydration); -setAttemptHydrationAtCurrentPriority(attemptHydrationAtCurrentPriority); -setGetCurrentUpdatePriority(getCurrentUpdatePriority); -setAttemptHydrationAtPriority(runWithPriority); - if (__DEV__) { if ( typeof Map !== 'function' || @@ -108,13 +82,6 @@ if (__DEV__) { } } -setRestoreImplementation(restoreControlledState); -setBatchingImplementation( - batchedUpdates, - discreteUpdates, - flushSyncWithoutWarningIfAlreadyRendering, -); - function createPortal( children: ReactNodeList, container: Element | DocumentFragment,