Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple event priority list from event name list #20760

Merged
merged 3 commits into from
Feb 8, 2021
Merged

Decouple event priority list from event name list #20760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b81b028
Remove some dead code
gaearon Feb 8, 2021
3fa516f
Decouple event priority list from event name list
gaearon Feb 8, 2021
1f8e642
Fix lint
gaearon Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 12 additions & 97 deletions packages/react-dom/src/events/DOMEventProperties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* @flow
*/

import type {EventPriority} from 'shared/ReactTypes';
import type {DOMEventName} from './DOMEventNames';

import {registerTwoPhaseEvent} from './EventRegistry';
Expand All @@ -17,7 +16,6 @@ import {
ANIMATION_START,
TRANSITION_END,
} from './DOMEventNames';
import {DiscreteEvent, ContinuousEvent, DefaultEvent} from 'shared/ReactTypes';

import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';

Expand All @@ -26,19 +24,8 @@ export const topLevelEventsToReactNames: Map<
string | null,
> = new Map();

const eventPriorities = new Map();

// We store most of the events in this module in pairs of two strings so we can re-use
// the code required to apply the same logic for event prioritization and that of the
// SimpleEventPlugin. This complicates things slightly, but the aim is to reduce code
// duplication (for which there would be quite a bit). For the events that are not needed
// for the SimpleEventPlugin (otherDiscreteEvents) we process them separately as an
// array of top level events.

// Lastly, we ignore prettier so we can keep the formatting sane.

// prettier-ignore
const discreteEventPairsForSimpleEventPlugin = [
const simpleEventPluginNames = [
('cancel': DOMEventName), 'cancel',
('click': DOMEventName), 'click',
('close': DOMEventName), 'close',
Expand Down Expand Up @@ -73,27 +60,7 @@ const discreteEventPairsForSimpleEventPlugin = [
('touchend': DOMEventName), 'touchEnd',
('touchstart': DOMEventName), 'touchStart',
('volumechange': DOMEventName), 'volumeChange',
];

const otherDiscreteEvents: Array<DOMEventName> = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These go away from this file because they're not SimpleEventPlugin events. They were only listed here to give them a discrete priority, which we do in the new switch.

'change',
'selectionchange',
'textInput',
'compositionstart',
'compositionend',
'compositionupdate',
];

if (enableCreateEventHandleAPI) {
// Special case: these two events don't have on* React handler
// and are only accessible via the createEventHandle API.
topLevelEventsToReactNames.set('beforeblur', null);
topLevelEventsToReactNames.set('afterblur', null);
otherDiscreteEvents.push('beforeblur', 'afterblur');
}

// prettier-ignore
const continuousPairsForSimpleEventPlugin: Array<string | DOMEventName> = [
('drag': DOMEventName), 'drag',
('dragenter': DOMEventName), 'dragEnter',
('dragexit': DOMEventName), 'dragExit',
Expand All @@ -109,10 +76,7 @@ const continuousPairsForSimpleEventPlugin: Array<string | DOMEventName> = [
('toggle': DOMEventName), 'toggle',
('touchmove': DOMEventName), 'touchMove',
('wheel': DOMEventName), 'wheel',
];

// prettier-ignore
const defaultPairsForSimpleEventPlugin: Array<string | DOMEventName> = [
('abort': DOMEventName), 'abort',
(ANIMATION_END: DOMEventName), 'animationEnd',
(ANIMATION_ITERATION: DOMEventName), 'animationIteration',
Expand Down Expand Up @@ -140,6 +104,13 @@ const defaultPairsForSimpleEventPlugin: Array<string | DOMEventName> = [
('waiting': DOMEventName), 'waiting',
];

if (enableCreateEventHandleAPI) {
// Special case: these two events don't have on* React handler
// and are only accessible via the createEventHandle API.
topLevelEventsToReactNames.set('beforeblur', null);
topLevelEventsToReactNames.set('afterblur', null);
}

/**
* Turns
* ['abort', ...]
Expand All @@ -152,75 +123,19 @@ const defaultPairsForSimpleEventPlugin: Array<string | DOMEventName> = [
*
* and registers them.
*/
function registerSimplePluginEventsAndSetTheirPriorities(
eventTypes: Array<DOMEventName | string>,
priority: EventPriority,
): void {
export function registerSimpleEvents() {
// As the event types are in pairs of two, we need to iterate
// through in twos. The events are in pairs of two to save code
// and improve init perf of processing this array, as it will
// result in far fewer object allocations and property accesses
// if we only use three arrays to process all the categories of
// instead of tuples.
for (let i = 0; i < eventTypes.length; i += 2) {
const topEvent = ((eventTypes[i]: any): DOMEventName);
const event = ((eventTypes[i + 1]: any): string);
for (let i = 0; i < simpleEventPluginNames.length; i += 2) {
const topEvent = ((simpleEventPluginNames[i]: any): DOMEventName);
const event = ((simpleEventPluginNames[i + 1]: any): string);
const capitalizedEvent = event[0].toUpperCase() + event.slice(1);
const reactName = 'on' + capitalizedEvent;
eventPriorities.set(topEvent, priority);
topLevelEventsToReactNames.set(topEvent, reactName);
registerTwoPhaseEvent(reactName, [topEvent]);
}
}

function setEventPriorities(
eventTypes: Array<DOMEventName>,
priority: EventPriority,
): void {
for (let i = 0; i < eventTypes.length; i++) {
eventPriorities.set(eventTypes[i], priority);
}
}

export function getEventPriorityForPluginSystem(
domEventName: DOMEventName,
): EventPriority {
const priority = eventPriorities.get(domEventName);
// Default to a DefaultEvent. Note: we might
// want to warn if we can't detect the priority
// for the event.
return priority === undefined ? DefaultEvent : priority;
}

export function getEventPriorityForListenerSystem(
type: DOMEventName,
): EventPriority {
const priority = eventPriorities.get(type);
if (priority !== undefined) {
return priority;
}
if (__DEV__) {
console.warn(
'The event "%s" provided to createEventHandle() does not have a known priority type.' +
' This is likely a bug in React.',
type,
);
}
return DefaultEvent;
}

export function registerSimpleEvents() {
registerSimplePluginEventsAndSetTheirPriorities(
discreteEventPairsForSimpleEventPlugin,
DiscreteEvent,
);
registerSimplePluginEventsAndSetTheirPriorities(
continuousPairsForSimpleEventPlugin,
ContinuousEvent,
);
registerSimplePluginEventsAndSetTheirPriorities(
defaultPairsForSimpleEventPlugin,
DefaultEvent,
);
setEventPriorities(otherDiscreteEvents, DiscreteEvent);
}
75 changes: 74 additions & 1 deletion packages/react-dom/src/events/ReactDOMEventListener.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {AnyNativeEvent} from '../events/PluginModuleType';
import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
import type {Container, SuspenseInstance} from '../client/ReactDOMHostConfig';
import type {DOMEventName} from '../events/DOMEventNames';
import type {EventPriority} from 'shared/ReactTypes';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to replace this with lane priority later. Conveniently, this file already imports them.


// Intentionally not named imports because Rollup would use dynamic dispatch for
// CommonJS interop named imports.
Expand Down Expand Up @@ -44,7 +45,6 @@ import {
enableNewReconciler,
} from 'shared/ReactFeatureFlags';
import {ContinuousEvent, DefaultEvent, DiscreteEvent} from 'shared/ReactTypes';
import {getEventPriorityForPluginSystem} from './DOMEventProperties';
import {dispatchEventForPluginEventSystem} from './DOMPluginEventSystem';
import {
flushDiscreteUpdatesIfNeeded,
Expand Down Expand Up @@ -339,3 +339,76 @@ export function attemptToDispatchEvent(
// We're not blocked on anything.
return null;
}

function getEventPriorityForPluginSystem(
domEventName: DOMEventName,
): EventPriority {
switch (domEventName) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copypasta from the list except that it doesn't need Default ones. Because they're ... default.

// Used by SimpleEventPlugin:
case 'cancel':
case 'click':
case 'close':
case 'contextmenu':
case 'copy':
case 'cut':
case 'auxclick':
case 'dblclick':
case 'dragend':
case 'dragstart':
case 'drop':
case 'focusin':
case 'focusout':
case 'input':
case 'invalid':
case 'keydown':
case 'keypress':
case 'keyup':
case 'mousedown':
case 'mouseup':
case 'paste':
case 'pause':
case 'play':
case 'pointercancel':
case 'pointerdown':
case 'pointerup':
case 'ratechange':
case 'reset':
case 'seeked':
case 'submit':
case 'touchcancel':
case 'touchend':
case 'touchstart':
case 'volumechange':
// Used by polyfills:
// eslint-disable-next-line no-fallthrough
case 'change':
case 'selectionchange':
case 'textInput':
case 'compositionstart':
case 'compositionend':
case 'compositionupdate':
// Only enableCreateEventHandleAPI:
// eslint-disable-next-line no-fallthrough
case 'beforeblur':
case 'afterblur':
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically these don't exist outside experimental but no real harm mentioning them.

return DiscreteEvent;
case 'drag':
case 'dragenter':
case 'dragexit':
case 'dragleave':
case 'dragover':
case 'mousemove':
case 'mouseout':
case 'mouseover':
case 'pointermove':
case 'pointerout':
case 'pointerover':
case 'scroll':
case 'toggle':
case 'touchmove':
case 'wheel':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

return ContinuousEvent;
default:
return DefaultEvent;
}
}
2 changes: 0 additions & 2 deletions packages/react-dom/src/events/ReactSyntheticEventType.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
*/

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import type {EventPriority} from 'shared/ReactTypes';
import type {DOMEventName} from './DOMEventNames';

export type DispatchConfig = {|
Expand All @@ -19,7 +18,6 @@ export type DispatchConfig = {|
captured: null | string,
|},
registrationName?: string,
eventPriority?: EventPriority,
|};

type BaseSyntheticEvent = {
Expand Down
2 changes: 0 additions & 2 deletions packages/react-native-renderer/src/legacy-events/ReactSyntheticEventType.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
*/

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import type {EventPriority} from 'shared/ReactTypes';
import type {TopLevelType} from './TopLevelEventTypes';

export type DispatchConfig = {|
Expand All @@ -19,7 +18,6 @@ export type DispatchConfig = {|
captured: null | string,
|},
registrationName?: string,
eventPriority: EventPriority,
|};

export type CustomDispatchConfig = {|
Expand Down