Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React Event System: cleanup plugins + break out update batching logic #18503

Merged
merged 1 commit into from Apr 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/legacy-events/EventPluginRegistry.js
Expand Up @@ -241,3 +241,20 @@ export function injectEventPluginsByName(
recomputePluginOrdering();
}
}

export function injectEventPlugins(
eventPlugins: [PluginModule<AnyNativeEvent>],
): void {
for (let i = 0; i < eventPlugins.length; i++) {
const pluginModule = eventPlugins[i];
plugins.push(pluginModule);
const publishedEvents = pluginModule.eventTypes;
for (const eventName in publishedEvents) {
publishEventForPlugin(
publishedEvents[eventName],
pluginModule,
eventName,
);
}
}
}
59 changes: 1 addition & 58 deletions packages/legacy-events/ReactGenericBatching.js
Expand Up @@ -5,14 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/

import {
needsStateRestore,
restoreStateIfNeeded,
} from './ReactControlledComponent';

import {enableDeprecatedFlareAPI} from 'shared/ReactFeatureFlags';
import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils';

// 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
Expand All @@ -32,21 +24,6 @@ let batchedEventUpdatesImpl = batchedUpdatesImpl;
let isInsideEventHandler = false;
let isBatchingEventUpdates = false;

function finishEventHandler() {
// Here we wait until all updates have propagated, which is important
// when using controlled components within layers:
// https://github.com/facebook/react/issues/1698
// Then we restore state of any controlled component.
const controlledComponentsHavePendingUpdates = needsStateRestore();
if (controlledComponentsHavePendingUpdates) {
// If a controlled event was fired, we may need to restore the state of
// the DOM node back to the controlled value. This is necessary when React
// bails out of the update without touching the DOM.
flushDiscreteUpdatesImpl();
restoreStateIfNeeded();
}
}

export function batchedUpdates(fn, bookkeeping) {
if (isInsideEventHandler) {
// If we are currently inside another batch, we need to wait until it
Expand All @@ -58,7 +35,6 @@ export function batchedUpdates(fn, bookkeeping) {
return batchedUpdatesImpl(fn, bookkeeping);
} finally {
isInsideEventHandler = false;
finishEventHandler();
}
}

Expand All @@ -73,19 +49,6 @@ export function batchedEventUpdates(fn, a, b) {
return batchedEventUpdatesImpl(fn, a, b);
} finally {
isBatchingEventUpdates = false;
finishEventHandler();
}
}

// This is for the React Flare event system
export function executeUserEventHandler(fn: any => void, value: any): void {
const previouslyInEventHandler = isInsideEventHandler;
try {
isInsideEventHandler = true;
const type = typeof value === 'object' && value !== null ? value.type : '';
invokeGuardedCallbackAndCatchFirstError(type, fn, undefined, value);
} finally {
isInsideEventHandler = previouslyInEventHandler;
}
}

Expand All @@ -97,32 +60,12 @@ export function discreteUpdates(fn, a, b, c, d) {
} finally {
isInsideEventHandler = prevIsInsideEventHandler;
if (!isInsideEventHandler) {
finishEventHandler();
}
}
}

let lastFlushedEventTimeStamp = 0;
export function flushDiscreteUpdatesIfNeeded(timeStamp: number) {
// event.timeStamp isn't overly reliable due to inconsistencies in
// how different browsers have historically provided the time stamp.
// Some browsers provide high-resolution time stamps for all events,
// some provide low-resolution time stamps for all events. FF < 52
// even mixes both time stamps together. Some browsers even report
// negative time stamps or time stamps that are 0 (iOS9) in some cases.
// Given we are only comparing two time stamps with equality (!==),
// we are safe from the resolution differences. If the time stamp is 0
// we bail-out of preventing the flush, which can affect semantics,
// such as if an earlier flush removes or adds event listeners that
// are fired in the subsequent flush. However, this is the same
// behaviour as we had before this change, so the risks are low.
if (
!isInsideEventHandler &&
(!enableDeprecatedFlareAPI ||
timeStamp === 0 ||
lastFlushedEventTimeStamp !== timeStamp)
) {
lastFlushedEventTimeStamp = timeStamp;
if (!isInsideEventHandler) {
flushDiscreteUpdatesImpl();
}
}
Expand Down
14 changes: 6 additions & 8 deletions packages/react-dom/src/client/ReactDOM.js
Expand Up @@ -39,13 +39,6 @@ import {
} from 'react-reconciler/src/ReactFiberReconciler';
import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
import {canUseDOM} from 'shared/ExecutionEnvironment';
import {setBatchingImplementation} from 'legacy-events/ReactGenericBatching';
import {
setRestoreImplementation,
enqueueStateRestore,
restoreStateIfNeeded,
} from 'legacy-events/ReactControlledComponent';
import {runEventsInBatch} from 'legacy-events/EventBatching';
import {
eventNameDispatchConfigs,
injectEventPluginsByName,
Expand All @@ -69,6 +62,12 @@ import {
setAttemptHydrationAtCurrentPriority,
queueExplicitHydrationTarget,
} from '../events/ReactDOMEventReplaying';
import {setBatchingImplementation} from '../events/ReactDOMUpdateBatching';
import {
setRestoreImplementation,
enqueueStateRestore,
restoreStateIfNeeded,
} from '../events/ReactDOMControlledComponent';

setAttemptSynchronousHydration(attemptSynchronousHydration);
setAttemptUserBlockingHydration(attemptUserBlockingHydration);
Expand Down Expand Up @@ -183,7 +182,6 @@ const Internals = {
enqueueStateRestore,
restoreStateIfNeeded,
dispatchEvent,
runEventsInBatch,
flushPassiveEffects,
IsThisRendererActing,
],
Expand Down
84 changes: 48 additions & 36 deletions packages/react-dom/src/client/ReactDOMClientInjection.js
Expand Up @@ -20,43 +20,55 @@ import SimpleEventPlugin from '../events/SimpleEventPlugin';
import {
injectEventPluginOrder,
injectEventPluginsByName,
injectEventPlugins,
} from 'legacy-events/EventPluginRegistry';
import {enableModernEventSystem} from 'shared/ReactFeatureFlags';

/**
* Specifies a deterministic ordering of `EventPlugin`s. A convenient way to
* reason about plugins, without having to package every one of them. This
* is better than having plugins be ordered in the same order that they
* are injected because that ordering would be influenced by the packaging order.
* `ResponderEventPlugin` must occur before `SimpleEventPlugin` so that
* preventing default on events is convenient in `SimpleEventPlugin` handlers.
*/
const DOMEventPluginOrder = [
'ResponderEventPlugin',
'SimpleEventPlugin',
'EnterLeaveEventPlugin',
'ChangeEventPlugin',
'SelectEventPlugin',
'BeforeInputEventPlugin',
];
if (enableModernEventSystem) {
injectEventPlugins([
SimpleEventPlugin,
EnterLeaveEventPlugin,
ChangeEventPlugin,
SelectEventPlugin,
BeforeInputEventPlugin,
]);
} else {
/**
* Specifies a deterministic ordering of `EventPlugin`s. A convenient way to
* reason about plugins, without having to package every one of them. This
* is better than having plugins be ordered in the same order that they
* are injected because that ordering would be influenced by the packaging order.
* `ResponderEventPlugin` must occur before `SimpleEventPlugin` so that
* preventing default on events is convenient in `SimpleEventPlugin` handlers.
*/
const DOMEventPluginOrder = [
'ResponderEventPlugin',
'SimpleEventPlugin',
'EnterLeaveEventPlugin',
'ChangeEventPlugin',
'SelectEventPlugin',
'BeforeInputEventPlugin',
];

/**
* Inject modules for resolving DOM hierarchy and plugin ordering.
*/
injectEventPluginOrder(DOMEventPluginOrder);
setComponentTree(
getFiberCurrentPropsFromNode,
getInstanceFromNode,
getNodeFromInstance,
);
/**
* Inject modules for resolving DOM hierarchy and plugin ordering.
*/
injectEventPluginOrder(DOMEventPluginOrder);
setComponentTree(
getFiberCurrentPropsFromNode,
getInstanceFromNode,
getNodeFromInstance,
);

/**
* Some important event plugins included by default (without having to require
* them).
*/
injectEventPluginsByName({
SimpleEventPlugin: SimpleEventPlugin,
EnterLeaveEventPlugin: EnterLeaveEventPlugin,
ChangeEventPlugin: ChangeEventPlugin,
SelectEventPlugin: SelectEventPlugin,
BeforeInputEventPlugin: BeforeInputEventPlugin,
});
/**
* Some important event plugins included by default (without having to require
* them).
*/
injectEventPluginsByName({
SimpleEventPlugin: SimpleEventPlugin,
EnterLeaveEventPlugin: EnterLeaveEventPlugin,
ChangeEventPlugin: ChangeEventPlugin,
SelectEventPlugin: SelectEventPlugin,
BeforeInputEventPlugin: BeforeInputEventPlugin,
});
}
16 changes: 12 additions & 4 deletions packages/react-dom/src/events/ChangeEventPlugin.js
Expand Up @@ -6,8 +6,6 @@
*/

import {runEventsInBatch} from 'legacy-events/EventBatching';
import {enqueueStateRestore} from 'legacy-events/ReactControlledComponent';
import {batchedUpdates} from 'legacy-events/ReactGenericBatching';
import SyntheticEvent from 'legacy-events/SyntheticEvent';
import isTextInputElement from './isTextInputElement';
import {canUseDOM} from 'shared/ExecutionEnvironment';
Expand All @@ -27,9 +25,15 @@ import isEventSupported from './isEventSupported';
import {getNodeFromInstance} from '../client/ReactDOMComponentTree';
import {updateValueIfChanged} from '../client/inputValueTracking';
import {setDefaultValue} from '../client/ReactDOMInput';
import {enqueueStateRestore} from './ReactDOMControlledComponent';

import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';
import {
disableInputAttributeSyncing,
enableModernEventSystem,
} from 'shared/ReactFeatureFlags';
import accumulateTwoPhaseListeners from './accumulateTwoPhaseListeners';
import {batchedUpdates} from './ReactDOMUpdateBatching';
import {dispatchEventsInBatch} from './DOMModernPluginEventSystem';

const eventTypes = {
change: {
Expand Down Expand Up @@ -101,7 +105,11 @@ function manualDispatchChangeEvent(nativeEvent) {
}

function runEventInBatch(event) {
runEventsInBatch(event);
if (enableModernEventSystem) {
dispatchEventsInBatch([event]);
} else {
runEventsInBatch(event);
}
}

function getInstIfValueChanged(targetInst) {
Expand Down
Expand Up @@ -22,7 +22,6 @@ import {
HostText,
} from 'react-reconciler/src/ReactWorkTags';
import {IS_FIRST_ANCESTOR, PLUGIN_EVENT_SYSTEM} from './EventSystemFlags';
import {batchedEventUpdates} from 'legacy-events/ReactGenericBatching';
import {runEventsInBatch} from 'legacy-events/EventBatching';
import {plugins} from 'legacy-events/EventPluginRegistry';
import accumulateInto from 'legacy-events/accumulateInto';
Expand All @@ -45,6 +44,7 @@ import {
mediaEventTypes,
} from './DOMTopLevelEventTypes';
import {addTrappedEventListener} from './ReactDOMEventListener';
import {batchedEventUpdates} from './ReactDOMUpdateBatching';

/**
* Summary of `DOMEventPluginSystem` event handling:
Expand Down
31 changes: 21 additions & 10 deletions packages/react-dom/src/events/DOMModernPluginEventSystem.js
Expand Up @@ -24,7 +24,6 @@ import type {
import type {ReactDOMListener} from '../shared/ReactDOMTypes';

import {registrationNameDependencies} from 'legacy-events/EventPluginRegistry';
import {batchedEventUpdates} from 'legacy-events/ReactGenericBatching';
import {plugins} from 'legacy-events/EventPluginRegistry';
import {
PLUGIN_EVENT_SYSTEM,
Expand Down Expand Up @@ -86,12 +85,16 @@ import {
} from '../client/ReactDOMComponentTree';
import {COMMENT_NODE} from '../shared/HTMLNodeType';
import {topLevelEventsToDispatchConfig} from './DOMEventProperties';
import {batchedEventUpdates} from './ReactDOMUpdateBatching';

import {
enableLegacyFBSupport,
enableUseEventAPI,
} from 'shared/ReactFeatureFlags';
import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils';
import {
invokeGuardedCallbackAndCatchFirstError,
rethrowCaughtError,
} from 'shared/ReactErrorUtils';

const capturePhaseEvents = new Set([
TOP_FOCUS,
Expand Down Expand Up @@ -213,6 +216,21 @@ function executeDispatchesInOrder(event: ReactSyntheticEvent): void {
event._dispatchCurrentTargets = null;
}

export function dispatchEventsInBatch(
events: Array<ReactSyntheticEvent>,
): void {
for (let i = 0; i < events.length; i++) {
const syntheticEvent = events[i];
executeDispatchesInOrder(syntheticEvent);
// Release the event from the pool if needed
if (!syntheticEvent.isPersistent()) {
syntheticEvent.constructor.release(syntheticEvent);
}
}
// This would be a good time to rethrow if any of the event handlers threw.
rethrowCaughtError();
}

function dispatchEventsForPlugins(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
Expand Down Expand Up @@ -244,14 +262,7 @@ function dispatchEventsForPlugins(
}
}
}
for (let i = 0; i < syntheticEvents.length; i++) {
const syntheticEvent = syntheticEvents[i];
executeDispatchesInOrder(syntheticEvent);
// Release the event from the pool if needed
if (!syntheticEvent.isPersistent()) {
syntheticEvent.constructor.release(syntheticEvent);
}
}
dispatchEventsInBatch(syntheticEvents);
}

function shouldUpgradeListener(
Expand Down
Expand Up @@ -30,13 +30,13 @@ import {
discreteUpdates,
flushDiscreteUpdatesIfNeeded,
executeUserEventHandler,
} from 'legacy-events/ReactGenericBatching';
import {enqueueStateRestore} from 'legacy-events/ReactControlledComponent';
} from './ReactDOMUpdateBatching';
import type {Fiber} from 'react-reconciler/src/ReactFiber';
import {enableDeprecatedFlareAPI} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';

import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree';
import {enqueueStateRestore} from './ReactDOMControlledComponent';
import {
ContinuousEvent,
UserBlockingEvent,
Expand Down