From f72f16a4c90983f2610a8d5ca0ca04ef29df6f7d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 22 Dec 2017 20:54:01 +0000 Subject: [PATCH 1/3] Refactor event emitters to reduce indirection --- packages/events/EventPluginHub.js | 42 ++++++++++--------- packages/events/ReactEventEmitterMixin.js | 36 ---------------- .../ResponderEventPlugin-test.internal.js | 3 +- .../react-dom/src/events/ChangeEventPlugin.js | 5 +-- .../src/events/ReactBrowserEventEmitter.js | 12 ++++-- .../src/test-utils/ReactTestUtils.js | 4 +- .../src/ReactNativeEventEmitter.js | 18 ++++---- 7 files changed, 45 insertions(+), 75 deletions(-) delete mode 100644 packages/events/ReactEventEmitterMixin.js diff --git a/packages/events/EventPluginHub.js b/packages/events/EventPluginHub.js index 48d7d6bc51e..ccf8afc87f9 100644 --- a/packages/events/EventPluginHub.js +++ b/packages/events/EventPluginHub.js @@ -164,13 +164,13 @@ export function getListener(inst: Fiber, registrationName: string) { * @return {*} An accumulation of synthetic events. * @internal */ -export function extractEvents( +function extractEvents( topLevelType: string, targetInst: Fiber, nativeEvent: AnyNativeEvent, nativeEventTarget: EventTarget, -) { - let events; +): Array | ReactSyntheticEvent | null { + let events = null; for (let i = 0; i < plugins.length; i++) { // Not every plugin in the ordering may be loaded at runtime. const possiblePlugin: PluginModule = plugins[i]; @@ -189,27 +189,14 @@ export function extractEvents( return events; } -/** - * Enqueues a synthetic event that should be dispatched when - * `processEventQueue` is invoked. - * - * @param {*} events An accumulation of synthetic events. - * @internal - */ -export function enqueueEvents( - events: Array | ReactSyntheticEvent, +export function runEventsInBatch( + events: Array | ReactSyntheticEvent | null, + simulated: boolean, ) { - if (events) { + if (events !== null) { eventQueue = accumulateInto(eventQueue, events); } -} -/** - * Dispatches all synthetic events on the event queue. - * - * @internal - */ -export function processEventQueue(simulated: boolean) { // Set `eventQueue` to null before processing it so that we can tell if more // events get enqueued while processing. const processingEventQueue = eventQueue; @@ -238,3 +225,18 @@ export function processEventQueue(simulated: boolean) { // This would be a good time to rethrow if any of the event handlers threw. ReactErrorUtils.rethrowCaughtError(); } + +export function handleTopLevel( + topLevelType: string, + targetInst: Fiber, + nativeEvent: AnyNativeEvent, + nativeEventTarget: EventTarget, +) { + const events = extractEvents( + topLevelType, + targetInst, + nativeEvent, + nativeEventTarget, + ); + runEventsInBatch(events, false); +} diff --git a/packages/events/ReactEventEmitterMixin.js b/packages/events/ReactEventEmitterMixin.js deleted file mode 100644 index 86bfe2e761a..00000000000 --- a/packages/events/ReactEventEmitterMixin.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Copyright (c) 2013-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - enqueueEvents, - processEventQueue, - extractEvents, -} from './EventPluginHub'; - -function runEventQueueInBatch(events) { - enqueueEvents(events); - processEventQueue(false); -} - -/** - * Streams a fired top-level event to `EventPluginHub` where plugins have the - * opportunity to create `ReactEvent`s to be dispatched. - */ -export function handleTopLevel( - topLevelType, - targetInst, - nativeEvent, - nativeEventTarget, -) { - const events = extractEvents( - topLevelType, - targetInst, - nativeEvent, - nativeEventTarget, - ); - runEventQueueInBatch(events); -} diff --git a/packages/events/__tests__/ResponderEventPlugin-test.internal.js b/packages/events/__tests__/ResponderEventPlugin-test.internal.js index 6a81f4c8768..d583126f0b1 100644 --- a/packages/events/__tests__/ResponderEventPlugin-test.internal.js +++ b/packages/events/__tests__/ResponderEventPlugin-test.internal.js @@ -312,8 +312,7 @@ const run = function(config, hierarchyConfig, nativeEventConfig) { // At this point the negotiation events have been dispatched as part of the // extraction process, but not the side effectful events. Below, we dispatch // side effectful events. - EventPluginHub.enqueueEvents(extractedEvents); - EventPluginHub.processEventQueue(true); + EventPluginHub.runEventsInBatch(extractedEvents, true); // Ensure that every event that declared an `order`, was actually dispatched. expect('number of events dispatched:' + runData.dispatchCount).toBe( diff --git a/packages/react-dom/src/events/ChangeEventPlugin.js b/packages/react-dom/src/events/ChangeEventPlugin.js index 392beab1be4..5e0dcbb3be4 100644 --- a/packages/react-dom/src/events/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/ChangeEventPlugin.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {enqueueEvents, processEventQueue} from 'events/EventPluginHub'; +import * as EventPluginHub from 'events/EventPluginHub'; import {accumulateTwoPhaseDispatches} from 'events/EventPropagators'; import {enqueueStateRestore} from 'events/ReactControlledComponent'; import {batchedUpdates} from 'events/ReactGenericBatching'; @@ -89,8 +89,7 @@ function manualDispatchChangeEvent(nativeEvent) { } function runEventInBatch(event) { - enqueueEvents(event); - processEventQueue(false); + EventPluginHub.runEventsInBatch(event, false); } function getInstIfValueChanged(targetInst) { diff --git a/packages/react-dom/src/events/ReactBrowserEventEmitter.js b/packages/react-dom/src/events/ReactBrowserEventEmitter.js index a9181b3fe9f..34dede9f0a6 100644 --- a/packages/react-dom/src/events/ReactBrowserEventEmitter.js +++ b/packages/react-dom/src/events/ReactBrowserEventEmitter.js @@ -6,7 +6,7 @@ */ import {registrationNameDependencies} from 'events/EventPluginRegistry'; - +import {handleTopLevel} from 'events/EventPluginHub'; import { setEnabled, isEnabled, @@ -16,8 +16,6 @@ import { import isEventSupported from './isEventSupported'; import BrowserEventConstants from './BrowserEventConstants'; -export * from 'events/ReactEventEmitterMixin'; - const {topLevelTypes} = BrowserEventConstants; /** @@ -163,4 +161,10 @@ export function isListeningToAllDependencies(registrationName, mountAt) { return true; } -export {setEnabled, isEnabled, trapBubbledEvent, trapCapturedEvent}; +export { + handleTopLevel, + setEnabled, + isEnabled, + trapBubbledEvent, + trapCapturedEvent, +}; diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index d32d8eca455..6062948c016 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -388,9 +388,7 @@ function makeSimulator(eventType) { // Normally extractEvent enqueues a state restore, but we'll just always // do that since we we're by-passing it here. ReactControlledComponent.enqueueStateRestore(domNode); - - EventPluginHub.enqueueEvents(event); - EventPluginHub.processEventQueue(true); + EventPluginHub.runEventsInBatch(event, true); }); }; } diff --git a/packages/react-native-renderer/src/ReactNativeEventEmitter.js b/packages/react-native-renderer/src/ReactNativeEventEmitter.js index c978f4e14ac..215b2d09de9 100644 --- a/packages/react-native-renderer/src/ReactNativeEventEmitter.js +++ b/packages/react-native-renderer/src/ReactNativeEventEmitter.js @@ -7,17 +7,21 @@ * @flow */ -import {getListener} from 'events/EventPluginHub'; +import {getListener, handleTopLevel} from 'events/EventPluginHub'; import {registrationNameModules} from 'events/EventPluginRegistry'; import {batchedUpdates} from 'events/ReactGenericBatching'; -import {handleTopLevel} from 'events/ReactEventEmitterMixin'; import warning from 'fbjs/lib/warning'; import {getInstanceFromNode} from './ReactNativeComponentTree'; import ReactNativeTagHandles from './ReactNativeTagHandles'; -export * from 'events/ReactEventEmitterMixin'; -export {getListener, registrationNameModules as registrationNames}; +import type {AnyNativeEvent} from 'events/PluginModuleType'; + +export { + handleTopLevel, + getListener, + registrationNameModules as registrationNames, +}; /** * Version of `ReactBrowserEventEmitter` that works on the receiving side of a @@ -25,7 +29,7 @@ export {getListener, registrationNameModules as registrationNames}; */ // Shared default empty native event - conserve memory. -const EMPTY_NATIVE_EVENT = {}; +const EMPTY_NATIVE_EVENT = (({}: any): AnyNativeEvent); /** * Selects a subsequence of `Touch`es, without destroying `touches`. @@ -90,7 +94,7 @@ const removeTouchesAtIndices = function( export function _receiveRootNodeIDEvent( rootNodeID: number, topLevelType: string, - nativeEventParam: ?Object, + nativeEventParam: ?AnyNativeEvent, ) { const nativeEvent = nativeEventParam || EMPTY_NATIVE_EVENT; const inst = getInstanceFromNode(rootNodeID); @@ -111,7 +115,7 @@ export function _receiveRootNodeIDEvent( export function receiveEvent( rootNodeID: number, topLevelType: string, - nativeEventParam: Object, + nativeEventParam: AnyNativeEvent, ) { _receiveRootNodeIDEvent(rootNodeID, topLevelType, nativeEventParam); } From 646d0815c22a6e797bead6eef1db4e25f9fcac58 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 22 Dec 2017 21:07:37 +0000 Subject: [PATCH 2/3] Remove unused handleTopLevel() injection --- packages/react-dom/src/client/ReactDOMClientInjection.js | 4 ---- packages/react-dom/src/events/ReactDOMEventListener.js | 8 ++------ 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMClientInjection.js b/packages/react-dom/src/client/ReactDOMClientInjection.js index cb4d2315f4c..10b3c00eebf 100644 --- a/packages/react-dom/src/client/ReactDOMClientInjection.js +++ b/packages/react-dom/src/client/ReactDOMClientInjection.js @@ -13,13 +13,9 @@ import BeforeInputEventPlugin from '../events/BeforeInputEventPlugin'; import ChangeEventPlugin from '../events/ChangeEventPlugin'; import DOMEventPluginOrder from '../events/DOMEventPluginOrder'; import EnterLeaveEventPlugin from '../events/EnterLeaveEventPlugin'; -import {handleTopLevel} from '../events/ReactBrowserEventEmitter'; -import {setHandleTopLevel} from '../events/ReactDOMEventListener'; import SelectEventPlugin from '../events/SelectEventPlugin'; import SimpleEventPlugin from '../events/SimpleEventPlugin'; -setHandleTopLevel(handleTopLevel); - /** * Inject modules for resolving DOM hierarchy and plugin ordering. */ diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 1fa4f67c3aa..451c03cdeb6 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -10,6 +10,7 @@ import {isFiberMounted} from 'react-reconciler/reflection'; import {HostRoot} from 'shared/ReactTypeOfWork'; import {addEventBubbleListener, addEventCaptureListener} from './EventListener'; +import {handleTopLevel} from './ReactBrowserEventEmitter'; import getEventTarget from './getEventTarget'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; @@ -85,7 +86,7 @@ function handleTopLevelImpl(bookKeeping) { for (let i = 0; i < bookKeeping.ancestors.length; i++) { targetInst = bookKeeping.ancestors[i]; - _handleTopLevel( + handleTopLevel( bookKeeping.topLevelType, targetInst, bookKeeping.nativeEvent, @@ -96,11 +97,6 @@ function handleTopLevelImpl(bookKeeping) { // TODO: can we stop exporting these? export let _enabled = true; -export let _handleTopLevel: null; - -export function setHandleTopLevel(handleTopLevel) { - _handleTopLevel = handleTopLevel; -} export function setEnabled(enabled) { _enabled = !!enabled; From 034775b919bdb7129122b9df2776edd1f7cabdf1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 22 Dec 2017 21:15:48 +0000 Subject: [PATCH 3/3] Rename handleTopLevel() to runExtractedEventsInBatch() and remove import indirection --- packages/events/EventPluginHub.js | 2 +- .../src/events/ReactBrowserEventEmitter.js | 9 +-------- .../react-dom/src/events/ReactDOMEventListener.js | 8 ++++---- .../src/ReactNativeEventEmitter.js | 15 ++++++++------- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/packages/events/EventPluginHub.js b/packages/events/EventPluginHub.js index ccf8afc87f9..d59ba35ff68 100644 --- a/packages/events/EventPluginHub.js +++ b/packages/events/EventPluginHub.js @@ -226,7 +226,7 @@ export function runEventsInBatch( ReactErrorUtils.rethrowCaughtError(); } -export function handleTopLevel( +export function runExtractedEventsInBatch( topLevelType: string, targetInst: Fiber, nativeEvent: AnyNativeEvent, diff --git a/packages/react-dom/src/events/ReactBrowserEventEmitter.js b/packages/react-dom/src/events/ReactBrowserEventEmitter.js index 34dede9f0a6..b0772c645d6 100644 --- a/packages/react-dom/src/events/ReactBrowserEventEmitter.js +++ b/packages/react-dom/src/events/ReactBrowserEventEmitter.js @@ -6,7 +6,6 @@ */ import {registrationNameDependencies} from 'events/EventPluginRegistry'; -import {handleTopLevel} from 'events/EventPluginHub'; import { setEnabled, isEnabled, @@ -161,10 +160,4 @@ export function isListeningToAllDependencies(registrationName, mountAt) { return true; } -export { - handleTopLevel, - setEnabled, - isEnabled, - trapBubbledEvent, - trapCapturedEvent, -}; +export {setEnabled, isEnabled, trapBubbledEvent, trapCapturedEvent}; diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 451c03cdeb6..515a0836641 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -6,11 +6,11 @@ */ import {batchedUpdates} from 'events/ReactGenericBatching'; +import {runExtractedEventsInBatch} from 'events/EventPluginHub'; import {isFiberMounted} from 'react-reconciler/reflection'; import {HostRoot} from 'shared/ReactTypeOfWork'; import {addEventBubbleListener, addEventCaptureListener} from './EventListener'; -import {handleTopLevel} from './ReactBrowserEventEmitter'; import getEventTarget from './getEventTarget'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; @@ -63,7 +63,7 @@ function releaseTopLevelCallbackBookKeeping(instance) { } } -function handleTopLevelImpl(bookKeeping) { +function handleTopLevel(bookKeeping) { let targetInst = bookKeeping.targetInst; // Loop through the hierarchy, in case there's any nested components. @@ -86,7 +86,7 @@ function handleTopLevelImpl(bookKeeping) { for (let i = 0; i < bookKeeping.ancestors.length; i++) { targetInst = bookKeeping.ancestors[i]; - handleTopLevel( + runExtractedEventsInBatch( bookKeeping.topLevelType, targetInst, bookKeeping.nativeEvent, @@ -176,7 +176,7 @@ export function dispatchEvent(topLevelType, nativeEvent) { try { // Event queue being processed in the same cycle allows // `preventDefault`. - batchedUpdates(handleTopLevelImpl, bookKeeping); + batchedUpdates(handleTopLevel, bookKeeping); } finally { releaseTopLevelCallbackBookKeeping(bookKeeping); } diff --git a/packages/react-native-renderer/src/ReactNativeEventEmitter.js b/packages/react-native-renderer/src/ReactNativeEventEmitter.js index 215b2d09de9..f75cbba0679 100644 --- a/packages/react-native-renderer/src/ReactNativeEventEmitter.js +++ b/packages/react-native-renderer/src/ReactNativeEventEmitter.js @@ -7,7 +7,7 @@ * @flow */ -import {getListener, handleTopLevel} from 'events/EventPluginHub'; +import {getListener, runExtractedEventsInBatch} from 'events/EventPluginHub'; import {registrationNameModules} from 'events/EventPluginRegistry'; import {batchedUpdates} from 'events/ReactGenericBatching'; import warning from 'fbjs/lib/warning'; @@ -17,11 +17,7 @@ import ReactNativeTagHandles from './ReactNativeTagHandles'; import type {AnyNativeEvent} from 'events/PluginModuleType'; -export { - handleTopLevel, - getListener, - registrationNameModules as registrationNames, -}; +export {getListener, registrationNameModules as registrationNames}; /** * Version of `ReactBrowserEventEmitter` that works on the receiving side of a @@ -99,7 +95,12 @@ export function _receiveRootNodeIDEvent( const nativeEvent = nativeEventParam || EMPTY_NATIVE_EVENT; const inst = getInstanceFromNode(rootNodeID); batchedUpdates(function() { - handleTopLevel(topLevelType, inst, nativeEvent, nativeEvent.target); + runExtractedEventsInBatch( + topLevelType, + inst, + nativeEvent, + nativeEvent.target, + ); }); // React Native doesn't use ReactControlledComponent but if it did, here's // where it would do it.