From 180cc5d80378b77199a2e65b4f83ce91c2196329 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 4 Jun 2019 13:35:22 -0700 Subject: [PATCH 1/2] Same as previous commit, but for Flare I don't know what the public API for setting the event priority should be. Right now it accepts a numeric enum, but is this what we want? Maybe it should be a string enum? I've punted on this for now. --- packages/events/ReactSyntheticEventType.js | 7 +-- .../src/events/DOMEventResponderSystem.js | 56 ++++++++++++++----- .../src/events/ReactDOMEventListener.js | 2 +- .../react-dom/src/events/SimpleEventPlugin.js | 8 +-- .../DOMEventResponderSystem-test.internal.js | 28 ++++++++-- packages/react-events/src/Drag.js | 22 +++++--- packages/react-events/src/Focus.js | 13 +++-- packages/react-events/src/Hover.js | 17 ++++-- packages/react-events/src/Press.js | 36 ++++++++---- packages/react-events/src/Swipe.js | 14 +++-- packages/shared/ReactTypes.js | 8 ++- 11 files changed, 147 insertions(+), 64 deletions(-) diff --git a/packages/events/ReactSyntheticEventType.js b/packages/events/ReactSyntheticEventType.js index dd2b6b55066a..71ae77fa0eda 100644 --- a/packages/events/ReactSyntheticEventType.js +++ b/packages/events/ReactSyntheticEventType.js @@ -9,14 +9,9 @@ */ import type {Fiber} from 'react-reconciler/src/ReactFiber'; +import type {EventPriority} from 'shared/ReactTypes'; import type {TopLevelType} from './TopLevelEventTypes'; -export opaque type EventPriority = 0 | 1 | 2; - -export const DiscreteEvent: EventPriority = 0; -export const UserBlockingEvent: EventPriority = 1; -export const ContinuousEvent: EventPriority = 2; - export type DispatchConfig = { dependencies: Array, phasedRegistrationNames?: { diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 3eb851db8726..6a19728470f1 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -23,6 +23,7 @@ import type { ReactEventComponentInstance, ReactResponderContext, ReactResponderEvent, + EventPriority, } from 'shared/ReactTypes'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; import { @@ -41,6 +42,20 @@ import { } from 'react-reconciler/src/ReactFiberEvents'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; +import { + ContinuousEvent, + UserBlockingEvent, + DiscreteEvent, +} from 'shared/ReactTypes'; +import {enableUserBlockingEvents} from 'shared/ReactFeatureFlags'; + +// Intentionally not named imports because Rollup would use dynamic dispatch for +// CommonJS interop named imports. +import * as Scheduler from 'scheduler'; +const { + unstable_UserBlockingPriority: UserBlockingPriority, + unstable_runWithPriority: runWithPriority, +} = Scheduler; export let listenToResponderEventTypesImpl; @@ -54,7 +69,7 @@ type EventObjectType = $Shape; type EventQueue = { events: Array, - discrete: boolean, + eventPriority: EventPriority, }; type PartialEventObject = { @@ -107,7 +122,7 @@ const eventResponderContext: ReactResponderContext = { dispatchEvent( possibleEventObject: Object, listener: ($Shape) => void, - discrete: boolean, + eventPriority: EventPriority, ): void { validateResponderContext(); const {target, type, timeStamp} = possibleEventObject; @@ -169,9 +184,7 @@ const eventResponderContext: ReactResponderContext = { PartialEventObject, >); const eventQueue = ((currentEventQueue: any): EventQueue); - if (discrete) { - eventQueue.discrete = true; - } + eventQueue.eventPriority = eventPriority; eventListeners.set(eventObject, listener); eventQueue.events.push(eventObject); }, @@ -585,7 +598,7 @@ function createResponderEvent( function createEventQueue(): EventQueue { return { events: [], - discrete: false, + eventPriority: ContinuousEvent, }; } @@ -604,18 +617,35 @@ function processEvents(events: Array): void { } export function processEventQueue(): void { - const {events, discrete} = ((currentEventQueue: any): EventQueue); + const {events, eventPriority} = ((currentEventQueue: any): EventQueue); if (events.length === 0) { return; } - if (discrete) { - flushDiscreteUpdatesIfNeeded(currentTimeStamp); - discreteUpdates(() => { + + switch (eventPriority) { + case DiscreteEvent: { + flushDiscreteUpdatesIfNeeded(currentTimeStamp); + discreteUpdates(() => { + batchedEventUpdates(processEvents, events); + }); + break; + } + case UserBlockingEvent: { + if (enableUserBlockingEvents) { + runWithPriority( + UserBlockingPriority, + batchedEventUpdates.bind(null, processEvents, events), + ); + } else { + batchedEventUpdates(processEvents, events); + } + break; + } + case ContinuousEvent: { batchedEventUpdates(processEvents, events); - }); - } else { - batchedEventUpdates(processEvents, events); + break; + } } } diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 378b857b5fdf..df77ec79e365 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -52,7 +52,7 @@ import { UserBlockingEvent, ContinuousEvent, DiscreteEvent, -} from 'events/ReactSyntheticEventType'; +} from 'shared/ReactTypes'; const { unstable_UserBlockingPriority: UserBlockingPriority, diff --git a/packages/react-dom/src/events/SimpleEventPlugin.js b/packages/react-dom/src/events/SimpleEventPlugin.js index 30649c67c263..c8798acda0ca 100644 --- a/packages/react-dom/src/events/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/SimpleEventPlugin.js @@ -7,6 +7,7 @@ * @flow */ +import type {EventPriority} from 'shared/ReactTypes'; import type { TopLevelType, DOMTopLevelEventType, @@ -14,18 +15,17 @@ import type { import type { DispatchConfig, ReactSyntheticEvent, - EventPriority, } from 'events/ReactSyntheticEventType'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type {EventTypes, PluginModule} from 'events/PluginModuleType'; -import {accumulateTwoPhaseDispatches} from 'events/EventPropagators'; -import SyntheticEvent from 'events/SyntheticEvent'; import { DiscreteEvent, UserBlockingEvent, ContinuousEvent, -} from 'events/ReactSyntheticEventType'; +} from 'shared/ReactTypes'; +import {accumulateTwoPhaseDispatches} from 'events/EventPropagators'; +import SyntheticEvent from 'events/SyntheticEvent'; import * as DOMTopLevelEventTypes from './DOMTopLevelEventTypes'; import warningWithoutStack from 'shared/warningWithoutStack'; 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 9a4dd5a6f716..f01e8045419a 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -13,6 +13,10 @@ let React; let ReactFeatureFlags; let ReactDOM; +// FIXME: What should the public API be for setting an event's priority? Right +// now it's an enum but is that what we want? Hard coding this for now. +const DiscreteEvent = 0; + function createReactEventComponent({ targetEventTypes, rootEventTypes, @@ -403,7 +407,11 @@ describe('DOMEventResponderSystem', () => { phase: 'bubble', timeStamp: context.getTimeStamp(), }; - context.dispatchEvent(syntheticEvent, props.onMagicClick, true); + context.dispatchEvent( + syntheticEvent, + props.onMagicClick, + DiscreteEvent, + ); } }, onEventCapture: (event, context, props) => { @@ -414,7 +422,11 @@ describe('DOMEventResponderSystem', () => { phase: 'capture', timeStamp: context.getTimeStamp(), }; - context.dispatchEvent(syntheticEvent, props.onMagicClick, true); + context.dispatchEvent( + syntheticEvent, + props.onMagicClick, + DiscreteEvent, + ); } }, }); @@ -456,7 +468,7 @@ describe('DOMEventResponderSystem', () => { phase, timeStamp: context.getTimeStamp(), }; - context.dispatchEvent(pressEvent, props.onPress, true); + context.dispatchEvent(pressEvent, props.onPress, DiscreteEvent); context.setTimeout(() => { if (props.onLongPress) { @@ -466,7 +478,11 @@ describe('DOMEventResponderSystem', () => { phase, timeStamp: context.getTimeStamp(), }; - context.dispatchEvent(longPressEvent, props.onLongPress, true); + context.dispatchEvent( + longPressEvent, + props.onLongPress, + DiscreteEvent, + ); } if (props.onLongPressChange) { @@ -479,7 +495,7 @@ describe('DOMEventResponderSystem', () => { context.dispatchEvent( longPressChangeEvent, props.onLongPressChange, - true, + DiscreteEvent, ); } }, 500); @@ -838,7 +854,7 @@ describe('DOMEventResponderSystem', () => { type: 'click', timeStamp: context.getTimeStamp(), }; - context.dispatchEvent(syntheticEvent, props.onClick, true); + context.dispatchEvent(syntheticEvent, props.onClick, DiscreteEvent); }, }); diff --git a/packages/react-events/src/Drag.js b/packages/react-events/src/Drag.js index fb13c54da181..68fd7b451ef5 100644 --- a/packages/react-events/src/Drag.js +++ b/packages/react-events/src/Drag.js @@ -10,9 +10,11 @@ import type { ReactResponderEvent, ReactResponderContext, + EventPriority, } from 'shared/ReactTypes'; import React from 'react'; +import {DiscreteEvent, UserBlockingEvent} from 'shared/ReactTypes'; const targetEventTypes = ['pointerdown']; const rootEventTypes = [ @@ -74,12 +76,12 @@ function dispatchDragEvent( name: DragEventType, listener: DragEvent => void, state: DragState, - discrete: boolean, + eventPriority: EventPriority, eventData?: EventData, ): void { const target = ((state.dragTarget: any): Element | Document); const syntheticEvent = createDragEvent(context, name, target, eventData); - context.dispatchEvent(syntheticEvent, listener, discrete); + context.dispatchEvent(syntheticEvent, listener, eventPriority); } const DragResponder = { @@ -130,7 +132,7 @@ const DragResponder = { 'dragstart', props.onDragStart, state, - true, + DiscreteEvent, ); } @@ -184,7 +186,7 @@ const DragResponder = { 'dragchange', dragChangeEventListener, state, - true, + UserBlockingEvent, ); } } else { @@ -203,7 +205,7 @@ const DragResponder = { 'dragmove', props.onDragMove, state, - false, + UserBlockingEvent, eventData, ); } @@ -222,7 +224,13 @@ const DragResponder = { context.releaseOwnership(); } if (props.onDragEnd) { - dispatchDragEvent(context, 'dragend', props.onDragEnd, state, true); + dispatchDragEvent( + context, + 'dragend', + props.onDragEnd, + state, + DiscreteEvent, + ); } if (props.onDragChange) { const dragChangeEventListener = () => { @@ -233,7 +241,7 @@ const DragResponder = { 'dragchange', dragChangeEventListener, state, - true, + UserBlockingEvent, ); } state.isDragging = false; diff --git a/packages/react-events/src/Focus.js b/packages/react-events/src/Focus.js index 82541ec291ec..9244fd2fcba8 100644 --- a/packages/react-events/src/Focus.js +++ b/packages/react-events/src/Focus.js @@ -13,6 +13,7 @@ import type { } from 'shared/ReactTypes'; import React from 'react'; +import {DiscreteEvent} from 'shared/ReactTypes'; type FocusProps = { disabled: boolean, @@ -97,7 +98,7 @@ function dispatchFocusInEvents( target, pointerType, ); - context.dispatchEvent(syntheticEvent, props.onFocus, true); + context.dispatchEvent(syntheticEvent, props.onFocus, DiscreteEvent); } if (props.onFocusChange) { const listener = () => { @@ -109,7 +110,7 @@ function dispatchFocusInEvents( target, pointerType, ); - context.dispatchEvent(syntheticEvent, listener, true); + context.dispatchEvent(syntheticEvent, listener, DiscreteEvent); } if (props.onFocusVisibleChange && state.isLocalFocusVisible) { const listener = () => { @@ -121,7 +122,7 @@ function dispatchFocusInEvents( target, pointerType, ); - context.dispatchEvent(syntheticEvent, listener, true); + context.dispatchEvent(syntheticEvent, listener, DiscreteEvent); } } @@ -139,7 +140,7 @@ function dispatchFocusOutEvents( target, pointerType, ); - context.dispatchEvent(syntheticEvent, props.onBlur, true); + context.dispatchEvent(syntheticEvent, props.onBlur, DiscreteEvent); } if (props.onFocusChange) { const listener = () => { @@ -151,7 +152,7 @@ function dispatchFocusOutEvents( target, pointerType, ); - context.dispatchEvent(syntheticEvent, listener, true); + context.dispatchEvent(syntheticEvent, listener, DiscreteEvent); } dispatchFocusVisibleOutEvent(context, props, state); } @@ -173,7 +174,7 @@ function dispatchFocusVisibleOutEvent( target, pointerType, ); - context.dispatchEvent(syntheticEvent, listener, true); + context.dispatchEvent(syntheticEvent, listener, DiscreteEvent); state.isLocalFocusVisible = false; } } diff --git a/packages/react-events/src/Hover.js b/packages/react-events/src/Hover.js index a44d06956e3c..9d557ab0ff9f 100644 --- a/packages/react-events/src/Hover.js +++ b/packages/react-events/src/Hover.js @@ -14,6 +14,7 @@ import type { import React from 'react'; import {isEventPositionWithinTouchHitTarget} from './utils'; +import {UserBlockingEvent} from 'shared/ReactTypes'; type HoverProps = { disabled: boolean, @@ -117,7 +118,7 @@ function dispatchHoverChangeEvent( 'hoverchange', ((state.hoverTarget: any): Element | Document), ); - context.dispatchEvent(syntheticEvent, listener, true); + context.dispatchEvent(syntheticEvent, listener, UserBlockingEvent); } function dispatchHoverStartEvents( @@ -155,7 +156,11 @@ function dispatchHoverStartEvents( 'hoverstart', ((target: any): Element | Document), ); - context.dispatchEvent(syntheticEvent, props.onHoverStart, true); + context.dispatchEvent( + syntheticEvent, + props.onHoverStart, + UserBlockingEvent, + ); } if (props.onHoverChange) { dispatchHoverChangeEvent(event, context, props, state); @@ -214,7 +219,11 @@ function dispatchHoverEndEvents( 'hoverend', ((target: any): Element | Document), ); - context.dispatchEvent(syntheticEvent, props.onHoverEnd, true); + context.dispatchEvent( + syntheticEvent, + props.onHoverEnd, + UserBlockingEvent, + ); } if (props.onHoverChange) { dispatchHoverChangeEvent(event, context, props, state); @@ -357,7 +366,7 @@ const HoverResponder = { context.dispatchEvent( syntheticEvent, props.onHoverMove, - true, + UserBlockingEvent, ); } } diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 189956d9d0be..b08b42293046 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -11,9 +11,11 @@ import type { ReactResponderEvent, ReactResponderContext, } from 'shared/ReactTypes'; +import type {EventPriority} from 'shared/ReactTypes'; import React from 'react'; import {isEventPositionWithinTouchHitTarget} from './utils'; +import {DiscreteEvent, UserBlockingEvent} from 'shared/ReactTypes'; type PressProps = { disabled: boolean, @@ -205,7 +207,7 @@ function dispatchEvent( state: PressState, name: PressEventType, listener: (e: Object) => void, - discrete: boolean, + eventPriority: EventPriority, ): void { const target = ((state.pressTarget: any): Element | Document); const pointerType = state.pointerType; @@ -216,7 +218,7 @@ function dispatchEvent( pointerType, event, ); - context.dispatchEvent(syntheticEvent, listener, discrete); + context.dispatchEvent(syntheticEvent, listener, eventPriority); } function dispatchPressChangeEvent( @@ -229,7 +231,7 @@ function dispatchPressChangeEvent( const listener = () => { props.onPressChange(bool); }; - dispatchEvent(event, context, state, 'presschange', listener, true); + dispatchEvent(event, context, state, 'presschange', listener, DiscreteEvent); } function dispatchLongPressChangeEvent( @@ -242,7 +244,14 @@ function dispatchLongPressChangeEvent( const listener = () => { props.onLongPressChange(bool); }; - dispatchEvent(event, context, state, 'longpresschange', listener, true); + dispatchEvent( + event, + context, + state, + 'longpresschange', + listener, + DiscreteEvent, + ); } function activate(event: ReactResponderEvent, context, props, state) { @@ -261,7 +270,7 @@ function activate(event: ReactResponderEvent, context, props, state) { state, 'pressstart', props.onPressStart, - true, + DiscreteEvent, ); } if (!wasActivePressed && props.onPressChange) { @@ -275,7 +284,14 @@ function deactivate(event: ?ReactResponderEvent, context, props, state) { state.isLongPressed = false; if (props.onPressEnd) { - dispatchEvent(event, context, state, 'pressend', props.onPressEnd, true); + dispatchEvent( + event, + context, + state, + 'pressend', + props.onPressEnd, + DiscreteEvent, + ); } if (props.onPressChange) { dispatchPressChangeEvent(event, context, props, state); @@ -321,7 +337,7 @@ function dispatchPressStartEvents( state, 'longpress', props.onLongPress, - true, + DiscreteEvent, ); } if (props.onLongPressChange) { @@ -683,7 +699,7 @@ const PressResponder = { state, 'contextmenu', props.onContextMenu, - true, + DiscreteEvent, ); } break; @@ -762,7 +778,7 @@ const PressResponder = { state, 'pressmove', props.onPressMove, - false, + UserBlockingEvent, ); } if ( @@ -844,7 +860,7 @@ const PressResponder = { state, 'press', props.onPress, - true, + DiscreteEvent, ); } } diff --git a/packages/react-events/src/Swipe.js b/packages/react-events/src/Swipe.js index b6404a2d028f..4a186af1f4c9 100644 --- a/packages/react-events/src/Swipe.js +++ b/packages/react-events/src/Swipe.js @@ -11,8 +11,10 @@ import type { ReactResponderEvent, ReactResponderContext, } from 'shared/ReactTypes'; +import type {EventPriority} from 'shared/ReactTypes'; import React from 'react'; +import {UserBlockingEvent, DiscreteEvent} from 'shared/ReactTypes'; const targetEventTypes = ['pointerdown']; const rootEventTypes = [ @@ -64,12 +66,12 @@ function dispatchSwipeEvent( name: SwipeEventType, listener: SwipeEvent => void, state: SwipeState, - discrete: boolean, + eventPriority: EventPriority, eventData?: EventData, ) { const target = ((state.swipeTarget: any): Element | Document); const syntheticEvent = createSwipeEvent(context, name, target, eventData); - context.dispatchEvent(syntheticEvent, listener, discrete); + context.dispatchEvent(syntheticEvent, listener, eventPriority); } type SwipeState = { @@ -197,7 +199,7 @@ const SwipeResponder = { 'swipemove', props.onSwipeMove, state, - false, + UserBlockingEvent, eventData, ); (nativeEvent: any).preventDefault(); @@ -226,7 +228,7 @@ const SwipeResponder = { 'swipeleft', props.onSwipeLeft, state, - true, + DiscreteEvent, ); } else if (props.onSwipeRight && direction === 1) { dispatchSwipeEvent( @@ -234,7 +236,7 @@ const SwipeResponder = { 'swiperight', props.onSwipeRight, state, - true, + DiscreteEvent, ); } } @@ -244,7 +246,7 @@ const SwipeResponder = { 'swipeend', props.onSwipeEnd, state, - true, + DiscreteEvent, ); } state.lastDirection = direction; diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index f109d2de92bb..5d7b32fe7d19 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -158,11 +158,17 @@ export type ReactResponderEvent = { passiveSupported: boolean, }; +export opaque type EventPriority = 0 | 1 | 2; + +export const DiscreteEvent: EventPriority = 0; +export const UserBlockingEvent: EventPriority = 1; +export const ContinuousEvent: EventPriority = 2; + export type ReactResponderContext = { dispatchEvent: ( eventObject: Object, listener: (Object) => void, - discrete: boolean, + eventPriority: EventPriority, ) => void, isTargetWithinElement: ( childTarget: Element | Document, From cbb943f73b1c6fd3d5431998f6a72eda7206052c Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 4 Jun 2019 14:00:43 -0700 Subject: [PATCH 2/2] Add test for hover events --- .../src/__tests__/Hover-test.internal.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/react-events/src/__tests__/Hover-test.internal.js b/packages/react-events/src/__tests__/Hover-test.internal.js index 9129cd2563f8..d60bd60b3fda 100644 --- a/packages/react-events/src/__tests__/Hover-test.internal.js +++ b/packages/react-events/src/__tests__/Hover-test.internal.js @@ -12,6 +12,8 @@ let React; let ReactFeatureFlags; let ReactDOM; +let TestUtils; +let Scheduler; let Hover; const createPointerEvent = (type, data) => { @@ -229,6 +231,48 @@ describe('Hover event responder', () => { expect(onHoverChange).toHaveBeenCalledTimes(2); expect(onHoverChange).toHaveBeenCalledWith(false); }); + + it('should be user-blocking but not discrete', async () => { + // This is currently behind a feature flag + jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableEventAPI = true; + ReactFeatureFlags.enableUserBlockingEvents = true; + React = require('react'); + ReactDOM = require('react-dom'); + TestUtils = require('react-dom/test-utils'); + Scheduler = require('scheduler'); + + const {act} = TestUtils; + const {useState} = React; + + const newContainer = document.createElement('div'); + document.body.appendChild(newContainer); + const root = ReactDOM.unstable_createRoot(newContainer); + + const target = React.createRef(null); + function Foo() { + const [isHover, setHover] = useState(false); + return ( + +
{isHover ? 'hovered' : 'not hovered'}
+ + ); + } + + await act(async () => { + root.render(); + }); + expect(newContainer.textContent).toEqual('not hovered'); + + await act(async () => { + target.current.dispatchEvent(createPointerEvent('mouseover')); + + // 3s should be enough to expire the updates + Scheduler.advanceTime(3000); + expect(newContainer.textContent).toEqual('hovered'); + }); + }); }); describe('onHoverEnd', () => {