Skip to content

Commit

Permalink
Remove capturePhaseEvents and separate events by bubbling
Browse files Browse the repository at this point in the history 
WIP

Refine all logic

Revise types
  • Loading branch information
@trueadm
trueadm committed Jul 8, 2020
1 parent dcb6032 commit 97e0106
Show file tree
Hide file tree
Showing 11 changed files with 269 additions and 351 deletions.
11 changes: 4 additions & 7 deletions packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,13 +349,10 @@ describe('ReactDOMEventListener', () => {
}),
);
// As of the modern event system refactor, we now support
// this on <img>. The reason for this, is because we now
// attach all media events to the "root" or "portal" in the
// capture phase, rather than the bubble phase. This allows
// us to assign less event listeners to individual elements,
// which also nicely allows us to support more without needing
// to add more individual code paths to support various
// events that do not bubble.
// this on <img>. The reason for this, is because we allow
// events to be attached to nodes regardless of if they
// necessary support them. This is a strange test, as this
// would never occur from normal browser behavior.
expect(handleImgLoadStart).toHaveBeenCalledTimes(1);

videoRef.current.dispatchEvent(
Expand Down
24 changes: 13 additions & 11 deletions packages/react-dom/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ import {
enableDeprecatedFlareAPI,
enableTrustedTypesIntegration,
} from 'shared/ReactFeatureFlags';
import {listenToReactPropEvent} from '../events/DOMModernPluginEventSystem';
import {listenToReactEvent} from '../events/DOMModernPluginEventSystem';
import {getEventListenerMap} from './ReactDOMComponentTree';

let didWarnInvalidHydration = false;
Expand Down Expand Up @@ -266,6 +266,7 @@ if (__DEV__) {
export function ensureListeningTo(
rootContainerInstance: Element | Node,
reactPropEvent: string,
targetElement?: Element,
): void {
// If we have a comment node, then use the parent node,
// which should be an element.
Expand All @@ -282,9 +283,10 @@ export function ensureListeningTo(
'ensureListeningTo(): received a container that was not an element node. ' +
'This is likely a bug in React.',
);
listenToReactPropEvent(
listenToReactEvent(
reactPropEvent,
((rootContainerElement: any): Element),
targetElement,
);
}

Expand Down Expand Up @@ -367,7 +369,7 @@ function setInitialDOMProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
ensureListeningTo(rootContainerElement, propKey);
ensureListeningTo(rootContainerElement, propKey, domElement);
}
} else if (nextProp != null) {
setValueForProperty(domElement, propKey, nextProp, isCustomComponentTag);
Expand Down Expand Up @@ -555,7 +557,7 @@ export function setInitialProperties(
props = ReactDOMInputGetHostProps(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange');
ensureListeningTo(rootContainerElement, 'onChange', domElement);
break;
case 'option':
ReactDOMOptionValidateProps(domElement, rawProps);
Expand All @@ -566,14 +568,14 @@ export function setInitialProperties(
props = ReactDOMSelectGetHostProps(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange');
ensureListeningTo(rootContainerElement, 'onChange', domElement);
break;
case 'textarea':
ReactDOMTextareaInitWrapperState(domElement, rawProps);
props = ReactDOMTextareaGetHostProps(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange');
ensureListeningTo(rootContainerElement, 'onChange', domElement);
break;
default:
props = rawProps;
Expand Down Expand Up @@ -793,7 +795,7 @@ export function diffProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
ensureListeningTo(rootContainerElement, propKey);
ensureListeningTo(rootContainerElement, propKey, domElement);
}
if (!updatePayload && lastProp !== nextProp) {
// This is a special case. If any listener updates we need to ensure
Expand Down Expand Up @@ -907,7 +909,7 @@ export function diffHydratedProperties(
ReactDOMInputInitWrapperState(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange');
ensureListeningTo(rootContainerElement, 'onChange', domElement);
break;
case 'option':
ReactDOMOptionValidateProps(domElement, rawProps);
Expand All @@ -916,13 +918,13 @@ export function diffHydratedProperties(
ReactDOMSelectInitWrapperState(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange');
ensureListeningTo(rootContainerElement, 'onChange', domElement);
break;
case 'textarea':
ReactDOMTextareaInitWrapperState(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange');
ensureListeningTo(rootContainerElement, 'onChange', domElement);
break;
}

Expand Down Expand Up @@ -989,7 +991,7 @@ export function diffHydratedProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
ensureListeningTo(rootContainerElement, propKey);
ensureListeningTo(rootContainerElement, propKey, domElement);
}
} else if (
__DEV__ &&
Expand Down
80 changes: 43 additions & 37 deletions packages/react-dom/src/client/ReactDOMEventHandle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,18 @@ import {
getClosestInstanceFromNode,
getEventHandlerListeners,
setEventHandlerListeners,
getEventListenerMap,
getFiberFromScopeInstance,
} from './ReactDOMComponentTree';
import {ELEMENT_NODE} from '../shared/HTMLNodeType';
import {
listenToTopLevelEvent,
listenToNativeEvent,
addEventTypeToDispatchConfig,
} from '../events/DOMModernPluginEventSystem';

import {HostRoot, HostPortal} from 'react-reconciler/src/ReactWorkTags';
import {
PLUGIN_EVENT_SYSTEM,
IS_TARGET_PHASE_ONLY,
EVENT_HANDLE_NON_MANAGED_NODE,
} from '../events/EventSystemFlags';

import {
Expand Down Expand Up @@ -84,9 +83,10 @@ function createEventHandleListener(
function registerEventOnNearestTargetContainer(
targetFiber: Fiber,
topLevelType: DOMTopLevelEventType,
passive: boolean | void,
isPassiveListener: boolean | void,
priority: EventPriority | void,
capture: boolean,
isCapturePhaseListener: boolean,
targetElement?: Element,
): void {
// If it is, find the nearest root or portal and make it
// our event handle target container.
Expand All @@ -98,24 +98,22 @@ function registerEventOnNearestTargetContainer(
'that did not have a corresponding root. This is likely a bug in React.',
);
}
const listenerMap = getEventListenerMap(targetContainer);
listenToTopLevelEvent(
listenToNativeEvent(
topLevelType,
isCapturePhaseListener,
targetContainer,
listenerMap,
PLUGIN_EVENT_SYSTEM,
capture,
passive,
targetElement,
isPassiveListener,
priority,
);
}

function registerReactDOMEvent(
target: EventTarget | ReactScopeInstance,
topLevelType: DOMTopLevelEventType,
passive: boolean | void,
capture: boolean,
priority: EventPriority | void,
isPassiveListener: boolean | void,
isCapturePhaseListener: boolean,
listenerPriority: EventPriority | void,
): void {
// Check if the target is a DOM element.
if ((target: any).nodeType === ELEMENT_NODE) {
Expand All @@ -132,9 +130,10 @@ function registerReactDOMEvent(
registerEventOnNearestTargetContainer(
targetFiber,
topLevelType,
passive,
priority,
capture,
isPassiveListener,
listenerPriority,
isCapturePhaseListener,
targetElement,
);
} else if (enableScopeAPI && isReactScope(target)) {
const scopeTarget = ((target: any): ReactScopeInstance);
Expand All @@ -146,21 +145,22 @@ function registerReactDOMEvent(
registerEventOnNearestTargetContainer(
targetFiber,
topLevelType,
passive,
priority,
capture,
isPassiveListener,
listenerPriority,
isCapturePhaseListener,
);
} else if (isValidEventTarget(target)) {
const eventTarget = ((target: any): EventTarget);
const listenerMap = getEventListenerMap(eventTarget);
listenToTopLevelEvent(
// These are valid event targets, but they are also
// non-managed React nodes.
listenToNativeEvent(
topLevelType,
isCapturePhaseListener,
eventTarget,
listenerMap,
PLUGIN_EVENT_SYSTEM | IS_TARGET_PHASE_ONLY,
capture,
passive,
priority,
undefined,
isPassiveListener,
listenerPriority,
PLUGIN_EVENT_SYSTEM | EVENT_HANDLE_NON_MANAGED_NODE,
);
} else {
invariant(
Expand All @@ -177,27 +177,27 @@ export function createEventHandle(
): ReactDOMEventHandle {
if (enableCreateEventHandleAPI) {
const topLevelType = ((type: any): DOMTopLevelEventType);
let capture = false;
let passive = undefined; // Undefined means to use the browser default
let priority;
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') {
capture = optionsCapture;
isCapturePhaseListener = optionsCapture;
}
if (typeof optionsPassive === 'boolean') {
passive = optionsPassive;
isPassiveListener = optionsPassive;
}
if (typeof optionsPriority === 'number') {
priority = optionsPriority;
listenerPriority = optionsPriority;
}
}
if (priority === undefined) {
priority = getEventPriorityForListenerSystem(topLevelType);
if (listenerPriority === undefined) {
listenerPriority = getEventPriorityForListenerSystem(topLevelType);
}

const registeredReactDOMEvents = new PossiblyWeakSet();
Expand All @@ -213,13 +213,19 @@ export function createEventHandle(
);
if (!registeredReactDOMEvents.has(target)) {
registeredReactDOMEvents.add(target);
registerReactDOMEvent(target, topLevelType, passive, capture, priority);
registerReactDOMEvent(
target,
topLevelType,
isPassiveListener,
isCapturePhaseListener,
listenerPriority,
);
// Add the event to our known event types list.
addEventTypeToDispatchConfig(topLevelType);
}
const listener = createEventHandleListener(
topLevelType,
capture,
isCapturePhaseListener,
callback,
);
let targetListeners = getEventHandlerListeners(target);
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ import {
} from 'shared/ReactFeatureFlags';
import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
import {TOP_BEFORE_BLUR, TOP_AFTER_BLUR} from '../events/DOMTopLevelEventTypes';
import {listenToReactPropEvent} from '../events/DOMModernPluginEventSystem';
import {listenToReactEvent} from '../events/DOMModernPluginEventSystem';

export type Type = string;
export type Props = {
Expand Down Expand Up @@ -1111,7 +1111,7 @@ export function makeOpaqueHydratingObject(
}

export function preparePortalMount(portalInstance: Instance): void {
listenToReactPropEvent('onMouseEnter', portalInstance);
listenToReactEvent('onMouseEnter', portalInstance);
}

export function prepareScopeUpdate(
Expand Down
Loading

0 comments on commit 97e0106

Please sign in to comment.