From e8e7a667a27ae70f9ce049a47730d1c8c809bb69 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 22 Dec 2017 18:17:36 +0000 Subject: [PATCH 1/2] Drop some top-level events from the list --- .../src/events/BrowserEventConstants.js | 28 ++----------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/packages/react-dom/src/events/BrowserEventConstants.js b/packages/react-dom/src/events/BrowserEventConstants.js index 475616889a7..3978a44407a 100644 --- a/packages/react-dom/src/events/BrowserEventConstants.js +++ b/packages/react-dom/src/events/BrowserEventConstants.js @@ -10,19 +10,16 @@ import getVendorPrefixedEventName from './getVendorPrefixedEventName'; /** * Types of raw signals from the browser caught at the top level. * - * For events like 'submit' which don't consistently bubble (which we - * trap at a lower node than `document`), binding at `document` would - * cause duplicate events so we don't include them here. + * For events like 'submit' or audio/video events which don't consistently + * bubble (which we trap at a lower node than `document`), binding + * at `document` would cause duplicate events so we don't include them here. */ const topLevelTypes = { - topAbort: 'abort', topAnimationEnd: getVendorPrefixedEventName('animationend'), topAnimationIteration: getVendorPrefixedEventName('animationiteration'), topAnimationStart: getVendorPrefixedEventName('animationstart'), topBlur: 'blur', topCancel: 'cancel', - topCanPlay: 'canplay', - topCanPlayThrough: 'canplaythrough', topChange: 'change', topClick: 'click', topClose: 'close', @@ -41,19 +38,12 @@ const topLevelTypes = { topDragOver: 'dragover', topDragStart: 'dragstart', topDrop: 'drop', - topDurationChange: 'durationchange', - topEmptied: 'emptied', - topEncrypted: 'encrypted', - topEnded: 'ended', - topError: 'error', topFocus: 'focus', topInput: 'input', topKeyDown: 'keydown', topKeyPress: 'keypress', topKeyUp: 'keyup', - topLoadedData: 'loadeddata', topLoad: 'load', - topLoadedMetadata: 'loadedmetadata', topLoadStart: 'loadstart', topMouseDown: 'mousedown', topMouseMove: 'mousemove', @@ -61,27 +51,15 @@ const topLevelTypes = { topMouseOver: 'mouseover', topMouseUp: 'mouseup', topPaste: 'paste', - topPause: 'pause', - topPlay: 'play', - topPlaying: 'playing', - topProgress: 'progress', - topRateChange: 'ratechange', topScroll: 'scroll', - topSeeked: 'seeked', - topSeeking: 'seeking', topSelectionChange: 'selectionchange', - topStalled: 'stalled', - topSuspend: 'suspend', topTextInput: 'textInput', - topTimeUpdate: 'timeupdate', topToggle: 'toggle', topTouchCancel: 'touchcancel', topTouchEnd: 'touchend', topTouchMove: 'touchmove', topTouchStart: 'touchstart', topTransitionEnd: getVendorPrefixedEventName('transitionend'), - topVolumeChange: 'volumechange', - topWaiting: 'waiting', topWheel: 'wheel', }; From 2dbf22efaf37ef9279b7e59b7213953f3b43ba2f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 22 Dec 2017 19:03:02 +0000 Subject: [PATCH 2/2] Put both whitelists in one file --- .../src/client/ReactDOMFiberComponent.js | 41 ++++--------------- .../src/events/BrowserEventConstants.js | 37 ++++++++++++++--- .../src/events/ReactBrowserEventEmitter.js | 4 +- .../src/test-utils/ReactTestUtils.js | 4 +- 4 files changed, 40 insertions(+), 46 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMFiberComponent.js b/packages/react-dom/src/client/ReactDOMFiberComponent.js index 6959351fe09..733432cd26e 100644 --- a/packages/react-dom/src/client/ReactDOMFiberComponent.js +++ b/packages/react-dom/src/client/ReactDOMFiberComponent.js @@ -22,6 +22,7 @@ import * as inputValueTracking from './inputValueTracking'; import setInnerHTML from './setInnerHTML'; import setTextContent from './setTextContent'; import {listenTo, trapBubbledEvent} from '../events/ReactBrowserEventEmitter'; +import {mediaEventTypes} from '../events/BrowserEventConstants'; import * as CSSPropertyOperations from '../shared/CSSPropertyOperations'; import {Namespaces, getIntrinsicNamespace} from '../shared/DOMNamespaces'; import { @@ -223,34 +224,6 @@ function getOwnerDocumentFromRootContainer( : rootContainerElement.ownerDocument; } -// There are so many media events, it makes sense to just -// maintain a list rather than create a `trapBubbledEvent` for each -const mediaEvents = { - topAbort: 'abort', - topCanPlay: 'canplay', - topCanPlayThrough: 'canplaythrough', - topDurationChange: 'durationchange', - topEmptied: 'emptied', - topEncrypted: 'encrypted', - topEnded: 'ended', - topError: 'error', - topLoadedData: 'loadeddata', - topLoadedMetadata: 'loadedmetadata', - topLoadStart: 'loadstart', - topPause: 'pause', - topPlay: 'play', - topPlaying: 'playing', - topProgress: 'progress', - topRateChange: 'ratechange', - topSeeked: 'seeked', - topSeeking: 'seeking', - topStalled: 'stalled', - topSuspend: 'suspend', - topTimeUpdate: 'timeupdate', - topVolumeChange: 'volumechange', - topWaiting: 'waiting', -}; - function trapClickOnNonInteractiveElement(node: HTMLElement) { // Mobile Safari does not fire properly bubble click events on // non-interactive elements, which means delegated click listeners do not @@ -472,9 +445,9 @@ export function setInitialProperties( case 'video': case 'audio': // Create listener for each media event - for (const event in mediaEvents) { - if (mediaEvents.hasOwnProperty(event)) { - trapBubbledEvent(event, mediaEvents[event], domElement); + for (const event in mediaEventTypes) { + if (mediaEventTypes.hasOwnProperty(event)) { + trapBubbledEvent(event, mediaEventTypes[event], domElement); } } props = rawProps; @@ -860,9 +833,9 @@ export function diffHydratedProperties( case 'video': case 'audio': // Create listener for each media event - for (const event in mediaEvents) { - if (mediaEvents.hasOwnProperty(event)) { - trapBubbledEvent(event, mediaEvents[event], domElement); + for (const event in mediaEventTypes) { + if (mediaEventTypes.hasOwnProperty(event)) { + trapBubbledEvent(event, mediaEventTypes[event], domElement); } } break; diff --git a/packages/react-dom/src/events/BrowserEventConstants.js b/packages/react-dom/src/events/BrowserEventConstants.js index 3978a44407a..203752892f6 100644 --- a/packages/react-dom/src/events/BrowserEventConstants.js +++ b/packages/react-dom/src/events/BrowserEventConstants.js @@ -14,7 +14,7 @@ import getVendorPrefixedEventName from './getVendorPrefixedEventName'; * bubble (which we trap at a lower node than `document`), binding * at `document` would cause duplicate events so we don't include them here. */ -const topLevelTypes = { +export const topLevelTypes = { topAnimationEnd: getVendorPrefixedEventName('animationend'), topAnimationIteration: getVendorPrefixedEventName('animationiteration'), topAnimationStart: getVendorPrefixedEventName('animationstart'), @@ -63,10 +63,35 @@ const topLevelTypes = { topWheel: 'wheel', }; -export type TopLevelTypes = $Enum; - -const BrowserEventConstants = { - topLevelTypes, +// There are so many media events, it makes sense to just +// maintain a list of them. Note these aren't technically +// "top-level" since they don't bubble. We should come up +// with a better naming convention if we come to refactoring +// the event system. +export const mediaEventTypes = { + topAbort: 'abort', + topCanPlay: 'canplay', + topCanPlayThrough: 'canplaythrough', + topDurationChange: 'durationchange', + topEmptied: 'emptied', + topEncrypted: 'encrypted', + topEnded: 'ended', + topError: 'error', + topLoadedData: 'loadeddata', + topLoadedMetadata: 'loadedmetadata', + topLoadStart: 'loadstart', + topPause: 'pause', + topPlay: 'play', + topPlaying: 'playing', + topProgress: 'progress', + topRateChange: 'ratechange', + topSeeked: 'seeked', + topSeeking: 'seeking', + topStalled: 'stalled', + topSuspend: 'suspend', + topTimeUpdate: 'timeupdate', + topVolumeChange: 'volumechange', + topWaiting: 'waiting', }; -export default BrowserEventConstants; +export type TopLevelTypes = $Enum; diff --git a/packages/react-dom/src/events/ReactBrowserEventEmitter.js b/packages/react-dom/src/events/ReactBrowserEventEmitter.js index b0772c645d6..8991734a750 100644 --- a/packages/react-dom/src/events/ReactBrowserEventEmitter.js +++ b/packages/react-dom/src/events/ReactBrowserEventEmitter.js @@ -13,9 +13,7 @@ import { trapCapturedEvent, } from './ReactDOMEventListener'; import isEventSupported from './isEventSupported'; -import BrowserEventConstants from './BrowserEventConstants'; - -const {topLevelTypes} = BrowserEventConstants; +import {topLevelTypes} from './BrowserEventConstants'; /** * Summary of `ReactBrowserEventEmitter` event handling: diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index 6062948c016..928a11836ec 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -18,7 +18,7 @@ import { import SyntheticEvent from 'events/SyntheticEvent'; import invariant from 'fbjs/lib/invariant'; -import BrowserEventConstants from '../events/BrowserEventConstants'; +import {topLevelTypes} from '../events/BrowserEventConstants'; const {findDOMNode} = ReactDOM; const { @@ -30,8 +30,6 @@ const { ReactDOMEventListener, } = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; -const topLevelTypes = BrowserEventConstants.topLevelTypes; - function Event(suffix) {} /**