Skip to content

Commit

Permalink
Undo dependency injection of batching (#26389)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage authored Mar 15, 2023
1 parent d310d65 commit a57f40d
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {
hasQueuedDiscreteEvents,
clearIfContinuousEvent,
queueIfContinuousEvent,
attemptSynchronousHydration,
} from './ReactDOMEventReplaying';
import {attemptSynchronousHydration} from 'react-reconciler/src/ReactFiberReconciler';
import {
getNearestMountedFiber,
getContainerFromFiber,
Expand Down
53 changes: 10 additions & 43 deletions packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: <T>(priority: EventPriority, fn: () => T) => T;

export function setAttemptHydrationAtPriority(
fn: <T>(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.
Expand Down
25 changes: 6 additions & 19 deletions packages/react-dom-bindings/src/events/ReactDOMUpdateBatching.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}
35 changes: 1 addition & 34 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -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' ||
Expand All @@ -108,13 +82,6 @@ if (__DEV__) {
}
}

setRestoreImplementation(restoreControlledState);
setBatchingImplementation(
batchedUpdates,
discreteUpdates,
flushSyncWithoutWarningIfAlreadyRendering,
);

function createPortal(
children: ReactNodeList,
container: Element | DocumentFragment,
Expand Down

0 comments on commit a57f40d

Please sign in to comment.