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

Jump to bottom

[Events] Make passiveness and priority non-configurable #19807

Merged
merged 1 commit into from
Sep 14, 2020
Merged

[Events] Make passiveness and priority non-configurable #19807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 5 additions & 16 deletions packages/react-dom/src/client/ReactDOMComponentTree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import type {
SuspenseInstance,
Props,
} from './ReactDOMHostConfig';
import type {DOMEventName} from '../events/DOMEventNames';

import {
HostComponent,
Expand All @@ -44,16 +43,6 @@ const internalEventHandlersKey = '__reactEvents$' + randomKey;
const internalEventHandlerListenersKey = '__reactListeners$' + randomKey;
const internalEventHandlesSetKey = '__reactHandles$' + randomKey;

export type ElementListenerMap = Map<
DOMEventName | string,
ElementListenerMapEntry | null,
>;

export type ElementListenerMapEntry = {
passive: void | boolean,
listener: any => void,
};

export function precacheFiberNode(
hostInst: Fiber,
node: Instance | TextInstance | SuspenseInstance | ReactScopeInstance,
Expand Down Expand Up @@ -207,12 +196,12 @@ export function updateFiberProps(
(node: any)[internalPropsKey] = props;
}

export function getEventListenerMap(node: EventTarget): ElementListenerMap {
let elementListenerMap = (node: any)[internalEventHandlersKey];
if (elementListenerMap === undefined) {
elementListenerMap = (node: any)[internalEventHandlersKey] = new Map();
export function getEventListenerSet(node: EventTarget): Set<string> {
let elementListenerSet = (node: any)[internalEventHandlersKey];
if (elementListenerSet === undefined) {
elementListenerSet = (node: any)[internalEventHandlersKey] = new Set();
}
return elementListenerMap;
return elementListenerSet;
}

export function getFiberFromScopeInstance(
Expand Down
173 changes: 69 additions & 104 deletions packages/react-dom/src/client/ReactDOMEventHandle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
*/

import type {DOMEventName} from '../events/DOMEventNames';
import type {EventPriority, ReactScopeInstance} from 'shared/ReactTypes';
import type {ReactScopeInstance} from 'shared/ReactTypes';
import type {
ReactDOMEventHandle,
ReactDOMEventHandleListener,
} from '../shared/ReactDOMTypes';

import {getEventPriorityForListenerSystem} from '../events/DOMEventProperties';
import {allNativeEvents} from '../events/EventRegistry';
import {
getClosestInstanceFromNode,
Expand All @@ -25,10 +24,7 @@ import {
addEventHandleToTarget,
} from './ReactDOMComponentTree';
import {ELEMENT_NODE, COMMENT_NODE} from '../shared/HTMLNodeType';
import {
listenToNativeEvent,
addEventTypeToDispatchConfig,
} from '../events/DOMPluginEventSystem';
import {listenToNativeEvent} from '../events/DOMPluginEventSystem';

import {HostRoot, HostPortal} from 'react-reconciler/src/ReactWorkTags';
import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../events/EventSystemFlags';
Expand All @@ -42,8 +38,6 @@ import invariant from 'shared/invariant';

type EventHandleOptions = {|
capture?: boolean,
passive?: boolean,
priority?: EventPriority,
|};

function getNearestRootOrPortalContainer(node: Fiber): null | Element {
Expand Down Expand Up @@ -82,76 +76,76 @@ function createEventHandleListener(
function registerEventOnNearestTargetContainer(
targetFiber: Fiber,
domEventName: DOMEventName,
isPassiveListener: boolean | void,
listenerPriority: EventPriority | void,
isCapturePhaseListener: boolean,
targetElement: Element | null,
): void {
// If it is, find the nearest root or portal and make it
// our event handle target container.
let targetContainer = getNearestRootOrPortalContainer(targetFiber);
if (targetContainer === null) {
invariant(
false,
'ReactDOM.createEventHandle: setListener called on an target ' +
'that did not have a corresponding root. This is likely a bug in React.',
if (!enableEagerRootListeners) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In eager mode, we attach to roots early, so no need to do it here anymore.

// If it is, find the nearest root or portal and make it
// our event handle target container.
let targetContainer = getNearestRootOrPortalContainer(targetFiber);
if (targetContainer === null) {
if (__DEV__) {
console.error(
'ReactDOM.createEventHandle: setListener called on an target ' +
'that did not have a corresponding root. This is likely a bug in React.',
);
}
return;
}
if (targetContainer.nodeType === COMMENT_NODE) {
targetContainer = ((targetContainer.parentNode: any): Element);
}
listenToNativeEvent(
domEventName,
isCapturePhaseListener,
targetContainer,
targetElement,
);
}
if (targetContainer.nodeType === COMMENT_NODE) {
targetContainer = ((targetContainer.parentNode: any): Element);
}
listenToNativeEvent(
domEventName,
isCapturePhaseListener,
targetContainer,
targetElement,
isPassiveListener,
listenerPriority,
);
}

function registerReactDOMEvent(
target: EventTarget | ReactScopeInstance,
domEventName: DOMEventName,
isPassiveListener: boolean | void,
isCapturePhaseListener: boolean,
listenerPriority: EventPriority | void,
): void {
// Check if the target is a DOM element.
if ((target: any).nodeType === ELEMENT_NODE) {
const targetElement = ((target: any): Element);
// Check if the DOM element is managed by React.
const targetFiber = getClosestInstanceFromNode(targetElement);
if (targetFiber === null) {
invariant(
false,
'ReactDOM.createEventHandle: setListener called on an element ' +
'target that is not managed by React. Ensure React rendered the DOM element.',
if (!enableEagerRootListeners) {
const targetElement = ((target: any): Element);
// Check if the DOM element is managed by React.
const targetFiber = getClosestInstanceFromNode(targetElement);
if (targetFiber === null) {
if (__DEV__) {
console.error(
'ReactDOM.createEventHandle: setListener called on an element ' +
'target that is not managed by React. Ensure React rendered the DOM element.',
);
}
return;
}
registerEventOnNearestTargetContainer(
targetFiber,
domEventName,
isCapturePhaseListener,
targetElement,
);
}
registerEventOnNearestTargetContainer(
targetFiber,
domEventName,
isPassiveListener,
listenerPriority,
isCapturePhaseListener,
targetElement,
);
} else if (enableScopeAPI && isReactScope(target)) {
const scopeTarget = ((target: any): ReactScopeInstance);
const targetFiber = getFiberFromScopeInstance(scopeTarget);
if (targetFiber === null) {
// Scope is unmounted, do not proceed.
return;
if (!enableEagerRootListeners) {
const scopeTarget = ((target: any): ReactScopeInstance);
const targetFiber = getFiberFromScopeInstance(scopeTarget);
if (targetFiber === null) {
// Scope is unmounted, do not proceed.
return;
}
registerEventOnNearestTargetContainer(
targetFiber,
domEventName,
isCapturePhaseListener,
null,
);
}
registerEventOnNearestTargetContainer(
targetFiber,
domEventName,
isPassiveListener,
listenerPriority,
isCapturePhaseListener,
null,
);
} else if (isValidEventTarget(target)) {
const eventTarget = ((target: any): EventTarget);
// These are valid event targets, but they are also
Expand All @@ -161,8 +155,6 @@ function registerReactDOMEvent(
isCapturePhaseListener,
eventTarget,
null,
isPassiveListener,
listenerPriority,
IS_EVENT_HANDLE_NON_MANAGED_NODE,
);
} else {
Expand All @@ -181,46 +173,27 @@ export function createEventHandle(
if (enableCreateEventHandleAPI) {
const domEventName = ((type: any): DOMEventName);

if (enableEagerRootListeners) {
// We cannot support arbitrary native events with eager root listeners
// because the eager strategy relies on knowing the whole list ahead of time.
// If we wanted to support this, we'd have to add code to keep track
// (or search) for all portal and root containers, and lazily add listeners
// to them whenever we see a previously unknown event. This seems like a lot
// of complexity for something we don't even have a particular use case for.
// Unfortunately, the downside of this invariant is that *removing* a native
// event from the list of known events has now become a breaking change for
// any code relying on the createEventHandle API.
invariant(
allNativeEvents.has(domEventName) ||
domEventName === 'beforeblur' ||
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've needed to put them into the list so that we actually listen to them.

domEventName === 'afterblur',
'Cannot call unstable_createEventHandle with "%s", as it is not an event known to React.',
domEventName,
);
}
// We cannot support arbitrary native events with eager root listeners
// because the eager strategy relies on knowing the whole list ahead of time.
// If we wanted to support this, we'd have to add code to keep track
// (or search) for all portal and root containers, and lazily add listeners
// to them whenever we see a previously unknown event. This seems like a lot
// of complexity for something we don't even have a particular use case for.
// Unfortunately, the downside of this invariant is that *removing* a native
// event from the list of known events has now become a breaking change for
// any code relying on the createEventHandle API.
invariant(
allNativeEvents.has(domEventName),
'Cannot call unstable_createEventHandle with "%s", as it is not an event known to React.',
domEventName,
);

let isCapturePhaseListener = false;
let isPassiveListener = undefined; // Undefined means to use the browser default
let listenerPriority;

if (options != null) {
const optionsCapture = options.capture;
const optionsPassive = options.passive;
const optionsPriority = options.priority;

if (typeof optionsCapture === 'boolean') {
isCapturePhaseListener = optionsCapture;
}
if (typeof optionsPassive === 'boolean') {
isPassiveListener = optionsPassive;
}
if (typeof optionsPriority === 'number') {
listenerPriority = optionsPriority;
}
}
if (listenerPriority === undefined) {
listenerPriority = getEventPriorityForListenerSystem(domEventName);
}

const eventHandle = (
Expand All @@ -234,15 +207,7 @@ export function createEventHandle(
);
if (!doesTargetHaveEventHandle(target, eventHandle)) {
addEventHandleToTarget(target, eventHandle);
registerReactDOMEvent(
target,
domEventName,
isPassiveListener,
isCapturePhaseListener,
listenerPriority,
);
// Add the event to our known event types list.
addEventTypeToDispatchConfig(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.

No need to do this now since we've removed support for arbitrary events for now.

registerReactDOMEvent(target, domEventName, isCapturePhaseListener);
}
const listener = createEventHandleListener(
domEventName,
Expand Down
6 changes: 5 additions & 1 deletion packages/react-dom/src/events/DOMEventProperties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ const otherDiscreteEvents: Array<DOMEventName> = [
];

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');
}

Expand Down Expand Up @@ -202,7 +206,7 @@ export function getEventPriorityForListenerSystem(
if (__DEV__) {
console.warn(
'The event "%s" provided to createEventHandle() does not have a known priority type.' +
' It is recommended to provide a "priority" option to specify a priority.',
' This is likely a bug in React.',
type,
);
}
Expand Down
Loading