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

Author: trueadm

packages/legacy-events/EventPluginRegistry.js

@@ -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,
+      );
+    }
+  }
+}

packages/legacy-events/ReactGenericBatching.js

@@ -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
@@ -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
@@ -58,7 +35,6 @@ export function batchedUpdates(fn, bookkeeping) {
     return batchedUpdatesImpl(fn, bookkeeping);
   } finally {
     isInsideEventHandler = false;
-    finishEventHandler();
   }
 }
 
@@ -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;
   }
 }
 
@@ -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();
   }
 }

packages/react-dom/src/client/ReactDOM.js

@@ -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,
@@ -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);
@@ -183,7 +182,6 @@ const Internals = {
     enqueueStateRestore,
     restoreStateIfNeeded,
     dispatchEvent,
-    runEventsInBatch,
     flushPassiveEffects,
     IsThisRendererActing,
   ],

packages/react-dom/src/client/ReactDOMClientInjection.js

@@ -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,
+  });
+}

packages/react-dom/src/events/ChangeEventPlugin.js

@@ -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';
@@ -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: {
@@ -101,7 +105,11 @@ function manualDispatchChangeEvent(nativeEvent) {
 }
 
 function runEventInBatch(event) {
-  runEventsInBatch(event);
+  if (enableModernEventSystem) {
+    dispatchEventsInBatch([event]);
+  } else {
+    runEventsInBatch(event);
+  }
 }
 
 function getInstIfValueChanged(targetInst) {

packages/react-dom/src/events/DOMLegacyEventPluginSystem.js

@@ -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';
@@ -45,6 +44,7 @@ import {
   mediaEventTypes,
 } from './DOMTopLevelEventTypes';
 import {addTrappedEventListener} from './ReactDOMEventListener';
+import {batchedEventUpdates} from './ReactDOMUpdateBatching';
 
 /**
  * Summary of `DOMEventPluginSystem` event handling:

packages/react-dom/src/events/DOMModernPluginEventSystem.js

@@ -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,
@@ -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,
@@ -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,
@@ -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(

packages/react-dom/src/events/DeprecatedDOMEventResponderSystem.js

@@ -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,