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

Fix

Fix conflicts

Fix flags

Fix
  • Loading branch information
@trueadm
trueadm committed Jul 8, 2020
1 parent d87220f commit c07ddfc
Show file tree
Hide file tree
Showing 10 changed files with 277 additions and 310 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,
IS_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 | IS_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 c07ddfc

Please sign in to comment.