From 9976db46c4ce070ed91f6a8ad37ed14c9cc3ffb2 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 2 Apr 2019 16:23:03 +0100 Subject: [PATCH 1/3] Refactor event object creation for the experimental event API --- packages/events/EventTypes.js | 29 +-- .../src/events/DOMEventResponderSystem.js | 173 ++++++++++++------ .../DOMEventResponderSystem-test.internal.js | 12 +- packages/react-events/src/Drag.js | 53 ++++-- packages/react-events/src/Focus.js | 42 ++++- packages/react-events/src/Hover.js | 46 ++++- packages/react-events/src/Press.js | 75 ++++---- packages/react-events/src/Swipe.js | 43 ++++- .../src/__tests__/Press-test.internal.js | 6 +- 9 files changed, 334 insertions(+), 145 deletions(-) diff --git a/packages/events/EventTypes.js b/packages/events/EventTypes.js index a03ef8f7405c1..077bb8f05bf3d 100644 --- a/packages/events/EventTypes.js +++ b/packages/events/EventTypes.js @@ -7,29 +7,32 @@ * @flow */ -import SyntheticEvent from 'events/SyntheticEvent'; import type {AnyNativeEvent} from 'events/PluginModuleType'; import type {ReactEventResponderEventType} from 'shared/ReactTypes'; +type ParitalEventObject = { + listener: ($Shape) => void, + target: Element | Document, + type: string, +}; + export type EventResponderContext = { event: AnyNativeEvent, - eventTarget: EventTarget, + eventTarget: Element | Document, eventType: string, isPassive: () => boolean, isPassiveSupported: () => boolean, - dispatchEvent: ( - name: string, - listener: (e: SyntheticEvent) => void | null, - pressTarget: EventTarget | null, + dispatchEvent: ( + eventObject: E, discrete: boolean, - extraProperties?: Object, + capture: boolean, ) => void, isTargetWithinElement: ( - childTarget: EventTarget, - parentTarget: EventTarget, + childTarget: Element | Document, + parentTarget: Element | Document, ) => boolean, - isTargetOwned: EventTarget => boolean, - isTargetWithinEventComponent: EventTarget => boolean, + isTargetOwned: (Element | Document) => boolean, + isTargetWithinEventComponent: (Element | Document) => boolean, isPositionWithinTouchHitTarget: (x: number, y: number) => boolean, addRootEventTypes: ( rootEventTypes: Array, @@ -37,6 +40,6 @@ export type EventResponderContext = { removeRootEventTypes: ( rootEventTypes: Array, ) => void, - requestOwnership: (target: EventTarget | null) => boolean, - releaseOwnership: (target: EventTarget | null) => boolean, + requestOwnership: (target: Element | Document | null) => boolean, + releaseOwnership: (target: Element | Document | null) => boolean, }; diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 9c52389654ffb..26a6f503b7162 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -18,15 +18,13 @@ import type { ReactEventResponderEventType, } from 'shared/ReactTypes'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; -import SyntheticEvent from 'events/SyntheticEvent'; -import {runEventsInBatch} from 'events/EventBatching'; -import {interactiveUpdates} from 'events/ReactGenericBatching'; -import {executeDispatch} from 'events/EventPluginUtils'; +import {batchedUpdates, interactiveUpdates} from 'events/ReactGenericBatching'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; - import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; import {enableEventAPI} from 'shared/ReactFeatureFlags'; +import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils'; +import warning from 'shared/warning'; let listenToResponderEventTypesImpl; @@ -46,11 +44,70 @@ const targetEventTypeCached: Map< > = new Map(); const targetOwnership: Map = new Map(); -type EventListener = (event: SyntheticEvent) => void; +type ParitalEventObject = { + listener: ($Shape) => void, + target: Element | Document, + type: string, +}; +type EventQueue = { + bubble: Array<$Shape>, + capture: Array<$Shape>, +}; +type BatchedEventQueue = { + discrete: null | EventQueue, + phase: EventQueuePhase, + nonDiscrete: null | EventQueue, +}; +type EventQueuePhase = 0 | 1; + +const DURING_EVENT_PHASE = 0; +const AFTER_EVENT_PHASE = 1; + +function createEventQueue(): EventQueue { + return { + bubble: [], + capture: [], + }; +} + +function createBatchedEventQueue(phase: EventQueuePhase): BatchedEventQueue { + return { + discrete: null, + phase, + nonDiscrete: null, + }; +} + +function processEvent(event: $Shape): void { + const type = event.type; + const listener = event.listener; + invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event); +} + +function processEventQueue(eventQueue: EventQueue): void { + const {bubble, capture} = eventQueue; + let i, length; -function copyEventProperties(eventData, syntheticEvent) { - for (let propName in eventData) { - syntheticEvent[propName] = eventData[propName]; + // TODO support stopPropagation via an alternative approach + // Process events in two phases + for (i = capture.length; i-- > 0; ) { + processEvent(capture[i]); + } + for (i = 0, length = bubble.length; i < length; ++i) { + processEvent(bubble[i]); + } +} + +function processBatchedEventQueue(batchedEventQueue: BatchedEventQueue): void { + const {discrete, nonDiscrete} = batchedEventQueue; + + if (discrete !== null) { + interactiveUpdates(() => { + processEventQueue(discrete); + }); + } + if (nonDiscrete !== null) { + processEventQueue(nonDiscrete); } } @@ -70,6 +127,7 @@ function DOMEventResponderContext( this._discreteEvents = null; this._nonDiscreteEvents = null; this._isBatching = true; + this._batchedEventQueue = createBatchedEventQueue(DURING_EVENT_PHASE); } DOMEventResponderContext.prototype.isPassive = function(): boolean { @@ -81,49 +139,58 @@ DOMEventResponderContext.prototype.isPassiveSupported = function(): boolean { }; DOMEventResponderContext.prototype.dispatchEvent = function( - eventName: string, - eventListener: EventListener, - eventTarget: AnyNativeEvent, + possibleEventObject: Object, discrete: boolean, - extraProperties?: Object, + capture: boolean, ): void { - const eventTargetFiber = getClosestInstanceFromNode(eventTarget); - const syntheticEvent = SyntheticEvent.getPooled( - null, - eventTargetFiber, - this.event, - eventTarget, - ); - if (extraProperties !== undefined) { - copyEventProperties(extraProperties, syntheticEvent); + const batchedEventQueue = this._batchedEventQueue; + const {listener, target, type} = possibleEventObject; + + if (listener == null || target == null || type == null) { + throw new Error( + 'context.dispatchEvent: "listener", "target" and "type" fields on event object are required.', + ); } - syntheticEvent.type = eventName; - syntheticEvent._dispatchInstances = [eventTargetFiber]; - syntheticEvent._dispatchListeners = [eventListener]; - - if (this._isBatching) { - let events; - if (discrete) { - events = this._discreteEvents; - if (events === null) { - events = this._discreteEvents = []; - } - } else { - events = this._nonDiscreteEvents; - if (events === null) { - events = this._nonDiscreteEvents = []; - } + if (__DEV__) { + possibleEventObject.preventDefault = () => { + // Update this warning when we have a story around dealing with preventDefault + warning( + false, + 'preventDefault() is no longer available on event objects created from event responder modules.', + ); + }; + possibleEventObject.stopPropagation = () => { + // Update this warning when we have a story around dealing with stopPropgation + warning( + false, + 'stopPropagation() is no longer available on event objects created from event responder modules.', + ); + }; + } + const eventObject = ((possibleEventObject: any): $Shape); + let eventQueue; + if (discrete) { + eventQueue = batchedEventQueue.discrete; + if (eventQueue === null) { + eventQueue = batchedEventQueue.discrete = createEventQueue(); } - events.push(syntheticEvent); } else { - if (discrete) { - interactiveUpdates(() => { - executeDispatch(syntheticEvent, eventListener, eventTargetFiber); - }); - } else { - executeDispatch(syntheticEvent, eventListener, eventTargetFiber); + eventQueue = batchedEventQueue.nonDiscrete; + if (eventQueue === null) { + eventQueue = batchedEventQueue.nonDiscrete = createEventQueue(); } } + let eventQueueArr; + if (capture) { + eventQueueArr = eventQueue.capture; + } else { + eventQueueArr = eventQueue.bubble; + } + eventQueueArr.push(eventObject); + + if (batchedEventQueue.phase === AFTER_EVENT_PHASE) { + batchedUpdates(processBatchedEventQueue, batchedEventQueue); + } }; DOMEventResponderContext.prototype.isTargetWithinEventComponent = function( @@ -318,17 +385,9 @@ export function runResponderEventsInBatch( ); } } - // Run batched events - const discreteEvents = context._discreteEvents; - if (discreteEvents !== null) { - interactiveUpdates(() => { - runEventsInBatch(discreteEvents); - }); - } - const nonDiscreteEvents = context._nonDiscreteEvents; - if (nonDiscreteEvents !== null) { - runEventsInBatch(nonDiscreteEvents); - } - context._isBatching = false; + processBatchedEventQueue(context._batchedEventQueue); + // In order to capture and process async events from responder modules + // we create a new event queue. + context._eventQueue = createBatchedEventQueue(AFTER_EVENT_PHASE); } } diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js index 4af4f210dffc1..992617e068e6d 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -228,12 +228,12 @@ describe('DOMEventResponderSystem', () => { ['click'], (context, props) => { if (props.onMagicClick) { - context.dispatchEvent( - 'magicclick', - props.onMagicClick, - context.eventTarget, - false, - ); + const event = { + listener: props.onMagicClick, + target: context.eventTarget, + type: 'magicclick', + }; + context.dispatchEvent(event, true, false); } }, ); diff --git a/packages/react-events/src/Drag.js b/packages/react-events/src/Drag.js index 7181b4efa233b..21c48a4ac9dbe 100644 --- a/packages/react-events/src/Drag.js +++ b/packages/react-events/src/Drag.js @@ -14,7 +14,7 @@ const targetEventTypes = ['pointerdown', 'pointercancel']; const rootEventTypes = ['pointerup', {name: 'pointermove', passive: false}]; type DragState = { - dragTarget: null | EventTarget, + dragTarget: null | Element | Document, isPointerDown: boolean, isDragging: boolean, startX: number, @@ -33,18 +33,45 @@ if (typeof window !== 'undefined' && window.PointerEvent === undefined) { }); } +type EventData = { + diffX: number, + diffY: number, +}; +type DragEventType = 'dragend' | 'dragchange' | 'dragmove'; + +type DragEvent = {| + listener: DragEvent => void, + target: Element | Document, + type: DragEventType, + diffX?: number, + diffY?: number, +|}; + +function createDragEvent( + type: DragEventType, + target: Element | Document, + listener: DragEvent => void, + eventData?: EventData, +): DragEvent { + return { + listener, + target, + type, + ...eventData, + }; +} + function dispatchDragEvent( context: EventResponderContext, - name: string, - listener: (e: Object) => void, + name: DragEventType, + listener: DragEvent => void, state: DragState, discrete: boolean, - eventData?: { - diffX: number, - diffY: number, - }, + eventData?: EventData, ): void { - context.dispatchEvent(name, listener, state.dragTarget, discrete, eventData); + const target = ((state.dragTarget: any): Element | Document); + const syntheticEvent = createDragEvent(name, target, listener, eventData); + context.dispatchEvent(syntheticEvent, discrete, false); } const DragResponder = { @@ -112,10 +139,11 @@ const DragResponder = { const dragChangeEventListener = () => { props.onDragChange(true); }; - context.dispatchEvent( + dispatchDragEvent( + context, 'dragchange', dragChangeEventListener, - state.dragTarget, + state, true, ); } @@ -160,10 +188,11 @@ const DragResponder = { const dragChangeEventListener = () => { props.onDragChange(false); }; - context.dispatchEvent( + dispatchDragEvent( + context, 'dragchange', dragChangeEventListener, - state.dragTarget, + state, true, ); } diff --git a/packages/react-events/src/Focus.js b/packages/react-events/src/Focus.js index 9c652f69d26be..f2672d580c2e6 100644 --- a/packages/react-events/src/Focus.js +++ b/packages/react-events/src/Focus.js @@ -19,24 +19,49 @@ type FocusState = { isFocused: boolean, }; +type FocusEventType = 'focus' | 'blur' | 'focuschange'; + +type FocusEvent = {| + listener: FocusEvent => void, + target: Element | Document, + type: FocusEventType, +|}; + +function createFocusEvent( + type: FocusEventType, + target: Element | Document, + listener: FocusEvent => void, +): FocusEvent { + return { + listener, + target, + type, + }; +} + function dispatchFocusInEvents(context: EventResponderContext, props: Object) { const {event, eventTarget} = context; if (context.isTargetWithinEventComponent((event: any).relatedTarget)) { return; } if (props.onFocus) { - context.dispatchEvent('focus', props.onFocus, eventTarget, true); + const syntheticEvent = createFocusEvent( + 'focus', + eventTarget, + props.onFocus, + ); + context.dispatchEvent(syntheticEvent, true, false); } if (props.onFocusChange) { const focusChangeEventListener = () => { props.onFocusChange(true); }; - context.dispatchEvent( + const syntheticEvent = createFocusEvent( 'focuschange', - focusChangeEventListener, eventTarget, - true, + focusChangeEventListener, ); + context.dispatchEvent(syntheticEvent, true, false); } } @@ -46,18 +71,19 @@ function dispatchFocusOutEvents(context: EventResponderContext, props: Object) { return; } if (props.onBlur) { - context.dispatchEvent('blur', props.onBlur, eventTarget, true); + const syntheticEvent = createFocusEvent('blur', eventTarget, props.onBlur); + context.dispatchEvent(syntheticEvent, true, false); } if (props.onFocusChange) { const focusChangeEventListener = () => { props.onFocusChange(false); }; - context.dispatchEvent( + const syntheticEvent = createFocusEvent( 'focuschange', - focusChangeEventListener, eventTarget, - true, + focusChangeEventListener, ); + context.dispatchEvent(syntheticEvent, true, false); } } diff --git a/packages/react-events/src/Hover.js b/packages/react-events/src/Hover.js index f8585fae8cdb3..74df35e32d295 100644 --- a/packages/react-events/src/Hover.js +++ b/packages/react-events/src/Hover.js @@ -23,6 +23,26 @@ type HoverState = { isTouched: boolean, }; +type HoverEventType = 'hover' | 'hoverstart' | 'hoverend' | 'hoverchange'; + +type HoverEvent = {| + listener: HoverEvent => void, + target: Element | Document, + type: HoverEventType, +|}; + +function createHoverEvent( + type: HoverEventType, + target: Element | Document, + listener: HoverEvent => void, +): HoverEvent { + return { + listener, + target, + type, + }; +} + // In the case we don't have PointerEvents (Safari), we listen to touch events // too if (typeof window !== 'undefined' && window.PointerEvent === undefined) { @@ -39,18 +59,23 @@ function dispatchHoverStartEvents( return; } if (props.onHoverStart) { - context.dispatchEvent('hoverstart', props.onHoverStart, eventTarget, true); + const syntheticEvent = createHoverEvent( + 'hoverstart', + eventTarget, + props.onHoverStart, + ); + context.dispatchEvent(syntheticEvent, true, false); } if (props.onHoverChange) { const hoverChangeEventListener = () => { props.onHoverChange(true); }; - context.dispatchEvent( + const syntheticEvent = createHoverEvent( 'hoverchange', - hoverChangeEventListener, eventTarget, - true, + hoverChangeEventListener, ); + context.dispatchEvent(syntheticEvent, true, false); } } @@ -60,18 +85,23 @@ function dispatchHoverEndEvents(context: EventResponderContext, props: Object) { return; } if (props.onHoverEnd) { - context.dispatchEvent('hoverend', props.onHoverEnd, eventTarget, true); + const syntheticEvent = createHoverEvent( + 'hoverend', + eventTarget, + props.onHoverEnd, + ); + context.dispatchEvent(syntheticEvent, true, false); } if (props.onHoverChange) { const hoverChangeEventListener = () => { props.onHoverChange(false); }; - context.dispatchEvent( + const syntheticEvent = createHoverEvent( 'hoverchange', - hoverChangeEventListener, eventTarget, - true, + hoverChangeEventListener, ); + context.dispatchEvent(syntheticEvent, true, false); } } diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index fe0310e4e0d26..fa4061b2de44e 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -36,13 +36,13 @@ type PressProps = { delayLongPress: number, delayPressEnd: number, delayPressStart: number, - onLongPress: (e: Object) => void, + onLongPress: (e: PressEvent) => void, onLongPressChange: boolean => void, onLongPressShouldCancelPress: () => boolean, - onPress: (e: Object) => void, + onPress: (e: PressEvent) => void, onPressChange: boolean => void, - onPressEnd: (e: Object) => void, - onPressStart: (e: Object) => void, + onPressEnd: (e: PressEvent) => void, + onPressStart: (e: PressEvent) => void, pressRententionOffset: Object, }; @@ -52,17 +52,45 @@ type PressState = { isLongPressed: boolean, isPressed: boolean, longPressTimeout: null | TimeoutID, - pressTarget: null | EventTarget, + pressTarget: null | Element | Document, shouldSkipMouseAfterTouch: boolean, }; +type PressEventType = + | 'press' + | 'pressstart' + | 'pressend' + | 'presschange' + | 'longpress' + | 'longpresschange'; + +type PressEvent = {| + listener: PressEvent => void, + target: Element | Document, + type: PressEventType, +|}; + +function createPressEvent( + type: PressEventType, + target: Element | Document, + listener: PressEvent => void, +): PressEvent { + return { + listener, + target, + type, + }; +} + function dispatchPressEvent( context: EventResponderContext, state: PressState, - name: string, + name: PressEventType, listener: (e: Object) => void, ): void { - context.dispatchEvent(name, listener, state.pressTarget, true); + const target = ((state.pressTarget: any): Element | Document); + const syntheticEvent = createPressEvent(name, target, listener); + context.dispatchEvent(syntheticEvent, true, false); } function dispatchPressStartEvents( @@ -105,9 +133,10 @@ function dispatchPressStartEvents( if (props.onLongPress) { const longPressEventListener = e => { props.onLongPress(e); - if (e.nativeEvent.defaultPrevented) { - state.defaultPrevented = true; - } + // TODO address this again at some point + // if (e.nativeEvent.defaultPrevented) { + // state.defaultPrevented = true; + // } }; dispatchPressEvent(context, state, 'longpress', longPressEventListener); } @@ -201,24 +230,7 @@ const PressResponder = { ) { return; } - let keyPressEventListener = props.onPress; - - // Wrap listener with prevent default behaviour, unless - // we are dealing with an anchor. Anchor tags are special beacuse - // we need to use the "click" event, to properly allow browser - // heuristics for cancelling link clicks. Furthermore, iOS and - // Android can show previous of anchor tags that requires working - // with click rather than touch events (and mouse down/up). - if (!isAnchorTagElement(eventTarget)) { - keyPressEventListener = e => { - if (!e.isDefaultPrevented() && !e.nativeEvent.defaultPrevented) { - e.preventDefault(); - state.defaultPrevented = true; - props.onPress(e); - } - }; - } - dispatchPressEvent(context, state, 'press', keyPressEventListener); + dispatchPressEvent(context, state, 'press', props.onPress); break; } @@ -340,9 +352,10 @@ const PressResponder = { ) { const pressEventListener = e => { props.onPress(e); - if (e.nativeEvent.defaultPrevented) { - state.defaultPrevented = true; - } + // TODO address this again at some point + // if (e.nativeEvent.defaultPrevented) { + // state.defaultPrevented = true; + // } }; dispatchPressEvent(context, state, 'press', pressEventListener); } diff --git a/packages/react-events/src/Swipe.js b/packages/react-events/src/Swipe.js index f265b2473242f..f7a17bcd8c3a8 100644 --- a/packages/react-events/src/Swipe.js +++ b/packages/react-events/src/Swipe.js @@ -23,18 +23,45 @@ if (typeof window !== 'undefined' && window.PointerEvent === undefined) { }); } +type EventData = { + diffX: number, + diffY: number, +}; +type SwipeEventType = 'swipeleft' | 'swiperight' | 'swipeend' | 'swipemove'; + +type SwipeEvent = {| + listener: SwipeEvent => void, + target: Element | Document, + type: SwipeEventType, + diffX?: number, + diffY?: number, +|}; + +function createSwipeEvent( + type: SwipeEventType, + target: Element | Document, + listener: SwipeEvent => void, + eventData?: EventData, +): SwipeEvent { + return { + listener, + target, + type, + ...eventData, + }; +} + function dispatchSwipeEvent( context: EventResponderContext, - name: string, - listener: (e: Object) => void, + name: SwipeEventType, + listener: SwipeEvent => void, state: SwipeState, discrete: boolean, - eventData?: { - diffX: number, - diffY: number, - }, + eventData?: EventData, ) { - context.dispatchEvent(name, listener, state.swipeTarget, discrete, eventData); + const target = ((state.swipeTarget: any): Element | Document); + const syntheticEvent = createSwipeEvent(name, target, listener, eventData); + context.dispatchEvent(syntheticEvent, discrete, false); } type SwipeState = { @@ -44,7 +71,7 @@ type SwipeState = { startX: number, startY: number, touchId: null | number, - swipeTarget: null | EventTarget, + swipeTarget: null | Element | Document, x: number, y: number, }; diff --git a/packages/react-events/src/__tests__/Press-test.internal.js b/packages/react-events/src/__tests__/Press-test.internal.js index 7ecdc3acb8671..e5575c78c3646 100644 --- a/packages/react-events/src/__tests__/Press-test.internal.js +++ b/packages/react-events/src/__tests__/Press-test.internal.js @@ -89,8 +89,10 @@ describe('Press event responder', () => { }); buttonRef.current.dispatchEvent(keyDownEvent); - // press 1 should not occur as press 2 will preventDefault - expect(events).toEqual(['keydown', 'press 2']); + // TODO update this test once we have a form of stopPropagation in + // the responder system again. This test had to be updated because + // we have removed stopPropagation() from synthetic events. + expect(events).toEqual(['keydown', 'press 2', 'press 1']); }); it('should support onPressStart and onPressEnd', () => { From f17a2f6aa1fcdbe123f161a1fd9ae4689e34f0cf Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 2 Apr 2019 19:46:23 +0100 Subject: [PATCH 2/3] Clean up of logic and added stopPropagation handling --- packages/events/EventTypes.js | 7 +- .../src/events/DOMEventResponderSystem.js | 121 ++++++++++-------- .../DOMEventResponderSystem-test.internal.js | 2 +- packages/react-events/src/Drag.js | 2 +- packages/react-events/src/Focus.js | 8 +- packages/react-events/src/Hover.js | 8 +- packages/react-events/src/Press.js | 2 +- packages/react-events/src/Swipe.js | 2 +- 8 files changed, 85 insertions(+), 67 deletions(-) diff --git a/packages/events/EventTypes.js b/packages/events/EventTypes.js index 077bb8f05bf3d..dc539e005b5dd 100644 --- a/packages/events/EventTypes.js +++ b/packages/events/EventTypes.js @@ -24,8 +24,11 @@ export type EventResponderContext = { isPassiveSupported: () => boolean, dispatchEvent: ( eventObject: E, - discrete: boolean, - capture: boolean, + { + capture?: boolean, + discrete?: boolean, + stopPropagation?: boolean, + }, ) => void, isTargetWithinElement: ( childTarget: Element | Document, diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 23a24057cf117..e0bc1c5c7abf2 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -34,6 +34,8 @@ export function setListenToResponderEventTypes( listenToResponderEventTypesImpl = _listenToResponderEventTypesImpl; } +const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set; + const rootEventTypesToEventComponents: Map< DOMTopLevelEventType | string, Set, @@ -43,6 +45,9 @@ const targetEventTypeCached: Map< Set, > = new Map(); const targetOwnership: Map = new Map(); +const eventsWithStopPropagation: + | WeakSet + | Set<$Shape> = new PossiblyWeakSet(); type ParitalEventObject = { listener: ($Shape) => void, @@ -50,31 +55,22 @@ type ParitalEventObject = { type: string, }; type EventQueue = { - bubble: Array<$Shape>, - capture: Array<$Shape>, -}; -type BatchedEventQueue = { - discrete: null | EventQueue, + bubble: null | Array<$Shape>, + capture: null | Array<$Shape>, + discrete: boolean, phase: EventQueuePhase, - nonDiscrete: null | EventQueue, }; type EventQueuePhase = 0 | 1; const DURING_EVENT_PHASE = 0; const AFTER_EVENT_PHASE = 1; -function createEventQueue(): EventQueue { - return { - bubble: [], - capture: [], - }; -} - -function createBatchedEventQueue(phase: EventQueuePhase): BatchedEventQueue { +function createEventQueue(phase: EventQueuePhase): EventQueue { return { - discrete: null, + bubble: null, + capture: null, + discrete: false, phase, - nonDiscrete: null, }; } @@ -84,30 +80,41 @@ function processEvent(event: $Shape): void { invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event); } -function processEventQueue(eventQueue: EventQueue): void { - const {bubble, capture} = eventQueue; +function processEvents( + bubble: null | Array<$Shape>, + capture: null | Array<$Shape>, +): void { let i, length; - // TODO support stopPropagation via an alternative approach - // Process events in two phases - for (i = capture.length; i-- > 0; ) { - processEvent(capture[i]); + if (capture !== null) { + for (i = capture.length; i-- > 0; ) { + const event = capture[i]; + processEvent(capture[i]); + if (eventsWithStopPropagation.has(event)) { + return; + } + } } - for (i = 0, length = bubble.length; i < length; ++i) { - processEvent(bubble[i]); + if (bubble !== null) { + for (i = 0, length = bubble.length; i < length; ++i) { + const event = bubble[i]; + processEvent(event); + if (eventsWithStopPropagation.has(event)) { + return; + } + } } } -function processBatchedEventQueue(batchedEventQueue: BatchedEventQueue): void { - const {discrete, nonDiscrete} = batchedEventQueue; +function processEventQueue(eventQueue: EventQueue): void { + const {bubble, capture, discrete} = eventQueue; - if (discrete !== null) { + if (discrete) { interactiveUpdates(() => { - processEventQueue(discrete); + processEvents(bubble, capture); }); - } - if (nonDiscrete !== null) { - processEventQueue(nonDiscrete); + } else { + processEvents(bubble, capture); } } @@ -127,7 +134,7 @@ function DOMEventResponderContext( this._discreteEvents = null; this._nonDiscreteEvents = null; this._isBatching = true; - this._batchedEventQueue = createBatchedEventQueue(DURING_EVENT_PHASE); + this._eventQueue = createEventQueue(DURING_EVENT_PHASE); } DOMEventResponderContext.prototype.isPassive = function(): boolean { @@ -140,10 +147,17 @@ DOMEventResponderContext.prototype.isPassiveSupported = function(): boolean { DOMEventResponderContext.prototype.dispatchEvent = function( possibleEventObject: Object, - discrete: boolean, - capture: boolean, + { + capture, + discrete, + stopPropagation, + }: { + capture?: boolean, + discrete?: boolean, + stopPropagation?: boolean, + }, ): void { - const batchedEventQueue = this._batchedEventQueue; + const eventQueue = this._eventQueue; const {listener, target, type} = possibleEventObject; if (listener == null || target == null || type == null) { @@ -168,28 +182,29 @@ DOMEventResponderContext.prototype.dispatchEvent = function( }; } const eventObject = ((possibleEventObject: any): $Shape); - let eventQueue; - if (discrete) { - eventQueue = batchedEventQueue.discrete; - if (eventQueue === null) { - eventQueue = batchedEventQueue.discrete = createEventQueue(); + let events; + + if (capture) { + events = eventQueue.capture; + if (events === null) { + events = eventQueue.capture = []; } } else { - eventQueue = batchedEventQueue.nonDiscrete; - if (eventQueue === null) { - eventQueue = batchedEventQueue.nonDiscrete = createEventQueue(); + events = eventQueue.bubble; + if (events === null) { + events = eventQueue.bubble = []; } } - let eventQueueArr; - if (capture) { - eventQueueArr = eventQueue.capture; - } else { - eventQueueArr = eventQueue.bubble; + if (discrete) { + eventQueue.discrete = true; } - eventQueueArr.push(eventObject); + events.push(eventObject); - if (batchedEventQueue.phase === AFTER_EVENT_PHASE) { - batchedUpdates(processBatchedEventQueue, batchedEventQueue); + if (stopPropagation) { + eventsWithStopPropagation.add(eventObject); + } + if (eventQueue.phase === AFTER_EVENT_PHASE) { + batchedUpdates(processEventQueue, eventQueue); } }; @@ -385,9 +400,9 @@ export function runResponderEventsInBatch( ); } } - processBatchedEventQueue(context._batchedEventQueue); + processEventQueue(context._eventQueue); // In order to capture and process async events from responder modules // we create a new event queue. - context._batchedEventQueue = createBatchedEventQueue(AFTER_EVENT_PHASE); + context._eventQueue = createEventQueue(AFTER_EVENT_PHASE); } } diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js index 992617e068e6d..b8ef52edf08e9 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -233,7 +233,7 @@ describe('DOMEventResponderSystem', () => { target: context.eventTarget, type: 'magicclick', }; - context.dispatchEvent(event, true, false); + context.dispatchEvent(event, {discrete: true}); } }, ); diff --git a/packages/react-events/src/Drag.js b/packages/react-events/src/Drag.js index 21c48a4ac9dbe..cc741e29b5b32 100644 --- a/packages/react-events/src/Drag.js +++ b/packages/react-events/src/Drag.js @@ -71,7 +71,7 @@ function dispatchDragEvent( ): void { const target = ((state.dragTarget: any): Element | Document); const syntheticEvent = createDragEvent(name, target, listener, eventData); - context.dispatchEvent(syntheticEvent, discrete, false); + context.dispatchEvent(syntheticEvent, {discrete}); } const DragResponder = { diff --git a/packages/react-events/src/Focus.js b/packages/react-events/src/Focus.js index f2672d580c2e6..c574f1e3a1f9b 100644 --- a/packages/react-events/src/Focus.js +++ b/packages/react-events/src/Focus.js @@ -50,7 +50,7 @@ function dispatchFocusInEvents(context: EventResponderContext, props: Object) { eventTarget, props.onFocus, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onFocusChange) { const focusChangeEventListener = () => { @@ -61,7 +61,7 @@ function dispatchFocusInEvents(context: EventResponderContext, props: Object) { eventTarget, focusChangeEventListener, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } } @@ -72,7 +72,7 @@ function dispatchFocusOutEvents(context: EventResponderContext, props: Object) { } if (props.onBlur) { const syntheticEvent = createFocusEvent('blur', eventTarget, props.onBlur); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onFocusChange) { const focusChangeEventListener = () => { @@ -83,7 +83,7 @@ function dispatchFocusOutEvents(context: EventResponderContext, props: Object) { eventTarget, focusChangeEventListener, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } } diff --git a/packages/react-events/src/Hover.js b/packages/react-events/src/Hover.js index 74df35e32d295..4d51bffae5e9b 100644 --- a/packages/react-events/src/Hover.js +++ b/packages/react-events/src/Hover.js @@ -64,7 +64,7 @@ function dispatchHoverStartEvents( eventTarget, props.onHoverStart, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onHoverChange) { const hoverChangeEventListener = () => { @@ -75,7 +75,7 @@ function dispatchHoverStartEvents( eventTarget, hoverChangeEventListener, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } } @@ -90,7 +90,7 @@ function dispatchHoverEndEvents(context: EventResponderContext, props: Object) { eventTarget, props.onHoverEnd, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onHoverChange) { const hoverChangeEventListener = () => { @@ -101,7 +101,7 @@ function dispatchHoverEndEvents(context: EventResponderContext, props: Object) { eventTarget, hoverChangeEventListener, ); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } } diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 2b2e8d80ffb69..3b7498153c977 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -90,7 +90,7 @@ function dispatchPressEvent( ): void { const target = ((state.pressTarget: any): Element | Document); const syntheticEvent = createPressEvent(name, target, listener); - context.dispatchEvent(syntheticEvent, true, false); + context.dispatchEvent(syntheticEvent, {discrete: true}); } function dispatchPressStartEvents( diff --git a/packages/react-events/src/Swipe.js b/packages/react-events/src/Swipe.js index f7a17bcd8c3a8..c67b502316e74 100644 --- a/packages/react-events/src/Swipe.js +++ b/packages/react-events/src/Swipe.js @@ -61,7 +61,7 @@ function dispatchSwipeEvent( ) { const target = ((state.swipeTarget: any): Element | Document); const syntheticEvent = createSwipeEvent(name, target, listener, eventData); - context.dispatchEvent(syntheticEvent, discrete, false); + context.dispatchEvent(syntheticEvent, {discrete}); } type SwipeState = { From 2b5dde8ade5fd131369201efa0021ecea9dca492 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 2 Apr 2019 19:48:29 +0100 Subject: [PATCH 3/3] Address feedback --- packages/events/EventTypes.js | 6 ------ .../src/events/DOMEventResponderSystem.js | 18 +++++++++--------- packages/react-events/src/Hover.js | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/events/EventTypes.js b/packages/events/EventTypes.js index dc539e005b5dd..b48052a6ad7a6 100644 --- a/packages/events/EventTypes.js +++ b/packages/events/EventTypes.js @@ -10,12 +10,6 @@ import type {AnyNativeEvent} from 'events/PluginModuleType'; import type {ReactEventResponderEventType} from 'shared/ReactTypes'; -type ParitalEventObject = { - listener: ($Shape) => void, - target: Element | Document, - type: string, -}; - export type EventResponderContext = { event: AnyNativeEvent, eventTarget: Element | Document, diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index e0bc1c5c7abf2..b4e982142b897 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -47,16 +47,16 @@ const targetEventTypeCached: Map< const targetOwnership: Map = new Map(); const eventsWithStopPropagation: | WeakSet - | Set<$Shape> = new PossiblyWeakSet(); + | Set<$Shape> = new PossiblyWeakSet(); -type ParitalEventObject = { - listener: ($Shape) => void, +type PartialEventObject = { + listener: ($Shape) => void, target: Element | Document, type: string, }; type EventQueue = { - bubble: null | Array<$Shape>, - capture: null | Array<$Shape>, + bubble: null | Array<$Shape>, + capture: null | Array<$Shape>, discrete: boolean, phase: EventQueuePhase, }; @@ -74,15 +74,15 @@ function createEventQueue(phase: EventQueuePhase): EventQueue { }; } -function processEvent(event: $Shape): void { +function processEvent(event: $Shape): void { const type = event.type; const listener = event.listener; invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event); } function processEvents( - bubble: null | Array<$Shape>, - capture: null | Array<$Shape>, + bubble: null | Array<$Shape>, + capture: null | Array<$Shape>, ): void { let i, length; @@ -181,7 +181,7 @@ DOMEventResponderContext.prototype.dispatchEvent = function( ); }; } - const eventObject = ((possibleEventObject: any): $Shape); + const eventObject = ((possibleEventObject: any): $Shape); let events; if (capture) { diff --git a/packages/react-events/src/Hover.js b/packages/react-events/src/Hover.js index 4d51bffae5e9b..75992d246aa17 100644 --- a/packages/react-events/src/Hover.js +++ b/packages/react-events/src/Hover.js @@ -23,7 +23,7 @@ type HoverState = { isTouched: boolean, }; -type HoverEventType = 'hover' | 'hoverstart' | 'hoverend' | 'hoverchange'; +type HoverEventType = 'hoverstart' | 'hoverend' | 'hoverchange'; type HoverEvent = {| listener: HoverEvent => void,