Do not bind topLevelType to dispatch (facebook#13618)

* 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. * Add note about why casting event.type to a topLevelType is safe * Move interactiveUpdates comment to point of assignment
Simek · Sep 14, 2018 · 0c9c591 · 0c9c591
1 parent 9f819a5
commit 0c9c591
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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,9 @@ function getTopLevelCallbackBookKeeping(
   targetInst: Fiber | null,
   ancestors: Array<Fiber>,
 } {
+  // This is safe because DOMTopLevelTypes are always native event type strings
+  const topLevelType = unsafeCastStringToDOMTopLevelType(nativeEvent.type);
+
   if (callbackBookkeepingPool.length) {
     const instance = callbackBookkeepingPool.pop();
     instance.topLevelType = topLevelType;
@@ -141,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.bind(null, topLevelType),
-  );
+  addEventBubbleListener(element, getRawEventName(topLevelType), dispatch);
 }
 
 /**
@@ -169,26 +169,20 @@ 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.bind(null, topLevelType),
-  );
+  addEventCaptureListener(element, getRawEventName(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 +201,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
@@ -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);
 }
 
 /**