From 7d2b2f1e39a9c8c5df45d56ff318ca39f3524324 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Tue, 11 Sep 2018 07:35:46 -0700 Subject: [PATCH 1/3] Do not bind topLevelType to dispatch A previous change made it such that all top level event types correspond to their associated native event string values. This commit eliminates the .bind attached to dispatch and fixes a related flow type. --- packages/events/PluginModuleType.js | 2 +- packages/events/ReactGenericBatching.js | 8 +++---- .../src/events/ReactDOMEventListener.js | 23 ++++++++----------- .../src/test-utils/ReactTestUtils.js | 6 +++-- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/packages/events/PluginModuleType.js b/packages/events/PluginModuleType.js index cd7a07661ab4e..0f79f749f5332 100644 --- a/packages/events/PluginModuleType.js +++ b/packages/events/PluginModuleType.js @@ -16,7 +16,7 @@ import type {TopLevelType} from './TopLevelEventTypes'; export type EventTypes = {[key: string]: DispatchConfig}; -export type AnyNativeEvent = Event | KeyboardEvent | MouseEvent | Touch; +export type AnyNativeEvent = Event | KeyboardEvent | MouseEvent | TouchEvent; export type PluginName = string; diff --git a/packages/events/ReactGenericBatching.js b/packages/events/ReactGenericBatching.js index 8347382f48a53..e37481e4deee8 100644 --- a/packages/events/ReactGenericBatching.js +++ b/packages/events/ReactGenericBatching.js @@ -20,8 +20,8 @@ import { let _batchedUpdatesImpl = function(fn, bookkeeping) { return fn(bookkeeping); }; -let _interactiveUpdatesImpl = function(fn, a, b) { - return fn(a, b); +let _interactiveUpdatesImpl = function(fn, a) { + return fn(a); }; let _flushInteractiveUpdatesImpl = function() {}; @@ -52,8 +52,8 @@ export function batchedUpdates(fn, bookkeeping) { } } -export function interactiveUpdates(fn, a, b) { - return _interactiveUpdatesImpl(fn, a, b); +export function interactiveUpdates(fn, a) { + return _interactiveUpdatesImpl(fn, a); } export function flushInteractiveUpdates() { diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index e49266d267f70..ff4bb717212bc 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -21,6 +21,7 @@ import getEventTarget from './getEventTarget'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; import SimpleEventPlugin from './SimpleEventPlugin'; import {getRawEventName} from './DOMTopLevelEventTypes'; +import {unsafeCastStringToDOMTopLevelType} from 'events/TopLevelEventTypes'; const {isInteractiveTopLevelEventType} = SimpleEventPlugin; @@ -48,7 +49,6 @@ function findRootContainerNode(inst) { // Used to store ancestor hierarchy in top level callback function getTopLevelCallbackBookKeeping( - topLevelType, nativeEvent, targetInst, ): { @@ -57,6 +57,8 @@ function getTopLevelCallbackBookKeeping( targetInst: Fiber | null, ancestors: Array, } { + const topLevelType = unsafeCastStringToDOMTopLevelType(nativeEvent.type); + if (callbackBookkeepingPool.length) { const instance = callbackBookkeepingPool.pop(); instance.topLevelType = topLevelType; @@ -149,7 +151,7 @@ export function trapBubbledEvent( element, getRawEventName(topLevelType), // Check if interactive and wrap in interactiveUpdates - dispatch.bind(null, topLevelType), + dispatch, ); } @@ -177,18 +179,15 @@ export function trapCapturedEvent( element, getRawEventName(topLevelType), // Check if interactive and wrap in interactiveUpdates - dispatch.bind(null, topLevelType), + dispatch, ); } -function dispatchInteractiveEvent(topLevelType, nativeEvent) { - interactiveUpdates(dispatchEvent, topLevelType, nativeEvent); +function dispatchInteractiveEvent(nativeEvent) { + interactiveUpdates(dispatchEvent, nativeEvent); } -export function dispatchEvent( - topLevelType: DOMTopLevelEventType, - nativeEvent: AnyNativeEvent, -) { +export function dispatchEvent(nativeEvent: AnyNativeEvent) { if (!_enabled) { return; } @@ -207,11 +206,7 @@ export function dispatchEvent( targetInst = null; } - const bookKeeping = getTopLevelCallbackBookKeeping( - topLevelType, - nativeEvent, - targetInst, - ); + const bookKeeping = getTopLevelCallbackBookKeeping(nativeEvent, targetInst); try { // Event queue being processed in the same cycle allows diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index dad3d0f14e2ef..88710e9518dc4 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -42,7 +42,9 @@ const [ runEventsInBatch, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; -function Event(suffix) {} +function Event(type) { + this.type = type; +} let hasWarnedAboutDeprecatedMockComponent = false; @@ -59,7 +61,7 @@ let hasWarnedAboutDeprecatedMockComponent = false; */ function simulateNativeEventOnNode(topLevelType, node, fakeNativeEvent) { fakeNativeEvent.target = node; - dispatchEvent(topLevelType, fakeNativeEvent); + dispatchEvent(fakeNativeEvent); } /** From 4e1f1a684d9bece5fd7f1beea607844270550c05 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Tue, 11 Sep 2018 11:57:12 -0700 Subject: [PATCH 2/3] Add note about why casting event.type to a topLevelType is safe --- packages/react-dom/src/events/ReactDOMEventListener.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index ff4bb717212bc..034c482260e72 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -57,6 +57,7 @@ function getTopLevelCallbackBookKeeping( targetInst: Fiber | null, ancestors: Array, } { + // This is safe because DOMTopLevelTypes are always native event type strings const topLevelType = unsafeCastStringToDOMTopLevelType(nativeEvent.type); if (callbackBookkeepingPool.length) { From 25819871a4e462e159edad9518b51e510e525645 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Tue, 11 Sep 2018 12:43:21 -0700 Subject: [PATCH 3/3] Move interactiveUpdates comment to point of assignment --- .../src/events/ReactDOMEventListener.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 034c482260e72..0ef3af801789a 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -144,16 +144,13 @@ export function trapBubbledEvent( if (!element) { return null; } + + // Check if interactive and wrap in interactiveUpdates const dispatch = isInteractiveTopLevelEventType(topLevelType) ? dispatchInteractiveEvent : dispatchEvent; - addEventBubbleListener( - element, - getRawEventName(topLevelType), - // Check if interactive and wrap in interactiveUpdates - dispatch, - ); + addEventBubbleListener(element, getRawEventName(topLevelType), dispatch); } /** @@ -172,16 +169,13 @@ export function trapCapturedEvent( if (!element) { return null; } + + // Check if interactive and wrap in interactiveUpdates const dispatch = isInteractiveTopLevelEventType(topLevelType) ? dispatchInteractiveEvent : dispatchEvent; - addEventCaptureListener( - element, - getRawEventName(topLevelType), - // Check if interactive and wrap in interactiveUpdates - dispatch, - ); + addEventCaptureListener(element, getRawEventName(topLevelType), dispatch); } function dispatchInteractiveEvent(nativeEvent) {