Address feedback

facebook · Mar 20, 2020 · 3f19be9 · 3f19be9
1 parent 7259765
commit 3f19be9
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 20 deletions.
diff --git a/packages/legacy-events/ReactSyntheticEventType.js b/packages/legacy-events/ReactSyntheticEventType.js
@@ -19,14 +19,21 @@ export type DispatchConfig = {|
     captured: null | string,
   |},
   registrationName?: string,
-  eventPriority?: EventPriority,
+  eventPriority: EventPriority,
+|};
+
+export type CustomDispatchConfig = {|
+  phasedRegistrationNames: {|
+    bubbled: null,
+    captured: null,
+  |},
   customEvent?: boolean,
 |};
 
 export type ReactSyntheticEvent = {|
-  dispatchConfig: DispatchConfig,
+  dispatchConfig: DispatchConfig | CustomDispatchConfig,
   getPooled: (
-    dispatchConfig: DispatchConfig,
+    dispatchConfig: DispatchConfig | CustomDispatchConfig,
     targetInst: Fiber,
     nativeTarget: Event,
     nativeEventTarget: EventTarget,

diff --git a/packages/react-dom/src/events/DOMEventProperties.js b/packages/react-dom/src/events/DOMEventProperties.js
@@ -12,7 +12,10 @@ import type {
   TopLevelType,
   DOMTopLevelEventType,
 } from 'legacy-events/TopLevelEventTypes';
-import type {DispatchConfig} from 'legacy-events/ReactSyntheticEventType';
+import type {
+  DispatchConfig,
+  CustomDispatchConfig,
+} from 'legacy-events/ReactSyntheticEventType';
 
 import * as DOMTopLevelEventTypes from './DOMTopLevelEventTypes';
 import {
@@ -31,7 +34,7 @@ export const simpleEventPluginEventTypes = {};
 
 export const topLevelEventsToDispatchConfig: Map<
   TopLevelType,
-  DispatchConfig,
+  DispatchConfig | CustomDispatchConfig,
 > = new Map();
 
 const eventPriorities = new Map();

diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js
@@ -17,7 +17,10 @@ import type {EventSystemFlags} from 'legacy-events/EventSystemFlags';
 import type {EventPriority} from 'shared/ReactTypes';
 import type {Fiber} from 'react-reconciler/src/ReactFiber';
 import type {PluginModule} from 'legacy-events/PluginModuleType';
-import type {ReactSyntheticEvent} from 'legacy-events/ReactSyntheticEventType';
+import type {
+  ReactSyntheticEvent,
+  CustomDispatchConfig,
+} from 'legacy-events/ReactSyntheticEventType';
 import type {ReactDOMListener} from 'shared/ReactDOMTypes';
 
 import {registrationNameDependencies} from 'legacy-events/EventPluginRegistry';
@@ -118,7 +121,8 @@ const capturePhaseEvents = new Set([
   TOP_VOLUME_CHANGE,
   TOP_WAITING,
 ]);
-const emptyDispatchConfigForCustomEvents = {
+
+const emptyDispatchConfigForCustomEvents: CustomDispatchConfig = {
   customEvent: true,
   phasedRegistrationNames: {
     bubbled: null,
@@ -434,7 +438,7 @@ export function attachElementListener(listener: ReactDOMListener): void {
   // If we don't have a dispatchConfig, then we're dealing with
   // an event type that React does not know about (i.e. a custom event).
   // We need to register an event config for this or the SimpleEventPlugin
-  // will not appropiately provide a SyntheticEvent, so we use out empty
+  // will not appropriately provide a SyntheticEvent, so we use out empty
   // dispatch config for custom events.
   if (dispatchConfig === undefined) {
     topLevelEventsToDispatchConfig.set(

diff --git a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js
@@ -1787,7 +1787,7 @@ describe('DOMModernPluginEventSystem', () => {
             expect(clickEvent).toHaveBeenCalledTimes(1);
           });
 
-          it('handle propagation of custom user events', () => {
+          it('handles propagation of custom user events', () => {
             const buttonRef = React.createRef();
             const divRef = React.createRef();
             const log = [];
@@ -1799,33 +1799,33 @@ describe('DOMModernPluginEventSystem', () => {
             );
 
             function Test() {
-              let custom;
+              let customEventHandle;
 
               // Test that we get a warning when we don't provide an explicit priortiy
               expect(() => {
-                custom = ReactDOM.unstable_useEvent('custom-event');
+                customEventHandle = ReactDOM.unstable_useEvent('custom-event');
               }).toWarnDev(
                 'Warning: The event "type" provided to useEvent() does not have a known priority type. ' +
                   'It is recommended to provide a "priority" option to specify a priority.',
               );
 
-              custom = ReactDOM.unstable_useEvent('custom-event', {
+              customEventHandle = ReactDOM.unstable_useEvent('custom-event', {
                 priority: 0, // Discrete
               });
 
-              const customCapture = ReactDOM.unstable_useEvent('custom-event', {
+              const customCaptureHandle = ReactDOM.unstable_useEvent('custom-event', {
                 capture: true,
                 priority: 0, // Discrete
               });
 
               React.useEffect(() => {
-                custom.setListener(buttonRef.current, onCustomEvent);
-                customCapture.setListener(
+                customEventHandle.setListener(buttonRef.current, onCustomEvent);
+                customCaptureHandle.setListener(
                   buttonRef.current,
                   onCustomEventCapture,
                 );
-                custom.setListener(divRef.current, onCustomEvent);
-                customCapture.setListener(divRef.current, onCustomEventCapture);
+                customEventHandle.setListener(divRef.current, onCustomEvent);
+                customCaptureHandle.setListener(divRef.current, onCustomEventCapture);
               });
 
               return (

diff --git a/packages/react-dom/src/events/accumulateTwoPhaseListeners.js b/packages/react-dom/src/events/accumulateTwoPhaseListeners.js
@@ -20,9 +20,9 @@ export default function accumulateTwoPhaseListeners(
   accumulateUseEventListeners?: boolean,
 ): void {
   const phasedRegistrationNames = event.dispatchConfig.phasedRegistrationNames;
-  const {bubbled, captured} = phasedRegistrationNames;
   const dispatchListeners = [];
   const dispatchInstances = [];
+  const {bubbled, captured} = phasedRegistrationNames;
   let node = event._targetInst;
 
   // Accumulate all instances and listeners via the target -> root path.
@@ -56,9 +56,8 @@ export default function accumulateTwoPhaseListeners(
           }
         }
       }
-      //
+      // Standard React on* listeners, i.e. onClick prop
       if (captured !== null) {
-        // Standard React on* listeners, i.e. onClick prop
         const captureListener = getListener(node, captured);
         if (captureListener != null) {
           // Capture listeners/instances should go at the start, so we