Skip to content

Commit

Permalink
Merge branch 'master' into effects_list_refactor_passive_effects
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Jul 22, 2020
2 parents c536679 + 356c171 commit 352b88b
Show file tree
Hide file tree
Showing 10 changed files with 305 additions and 218 deletions.
66 changes: 53 additions & 13 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 All @@ -374,7 +371,9 @@ describe('ReactDOMEventListener', () => {
document.body.appendChild(container);

const videoRef = React.createRef();
const handleVideoPlay = jest.fn(); // We'll test this one.
// We'll test this event alone.
const handleVideoPlay = jest.fn();
const handleVideoPlayDelegated = jest.fn();
const mediaEvents = {
onAbort() {},
onCanPlay() {},
Expand All @@ -401,23 +400,32 @@ describe('ReactDOMEventListener', () => {
onWaiting() {},
};

const originalAddEventListener = document.addEventListener;
const originalDocAddEventListener = document.addEventListener;
const originalRootAddEventListener = container.addEventListener;
document.addEventListener = function(type) {
throw new Error(
`Did not expect to add a top-level listener for the "${type}" event.`,
`Did not expect to add a document-level listener for the "${type}" event.`,
);
};
container.addEventListener = function(type) {
if (type === 'mouseout' || type === 'mouseover') {
// We currently listen to it unconditionally.
return;
}
throw new Error(
`Did not expect to add a root-level listener for the "${type}" event.`,
);
};

try {
// We expect that mounting this tree will
// *not* attach handlers for any top-level events.
ReactDOM.render(
<div>
<div onPlay={handleVideoPlayDelegated}>
<video ref={videoRef} {...mediaEvents} onPlay={handleVideoPlay} />
<audio {...mediaEvents}>
<source {...mediaEvents} />
</audio>
<form onReset={() => {}} onSubmit={() => {}} />
</div>,
container,
);
Expand All @@ -429,8 +437,12 @@ describe('ReactDOMEventListener', () => {
}),
);
expect(handleVideoPlay).toHaveBeenCalledTimes(1);
// Unlike browsers, we delegate media events.
// (This doesn't make a lot of sense but it would be a breaking change not to.)
expect(handleVideoPlayDelegated).toHaveBeenCalledTimes(1);
} finally {
document.addEventListener = originalAddEventListener;
document.addEventListener = originalDocAddEventListener;
container.addEventListener = originalRootAddEventListener;
document.body.removeChild(container);
}
});
Expand Down Expand Up @@ -461,4 +473,32 @@ describe('ReactDOMEventListener', () => {
document.body.removeChild(container);
}
});

// Unlike browsers, we delegate media events.
// (This doesn't make a lot of sense but it would be a breaking change not to.)
it('should delegate media events even without a direct listener', () => {
const container = document.createElement('div');
const ref = React.createRef();
const handleVideoPlayDelegated = jest.fn();
document.body.appendChild(container);
try {
ReactDOM.render(
<div onPlay={handleVideoPlayDelegated}>
{/* Intentionally no handler on the target: */}
<video ref={ref} />
</div>,
container,
);
ref.current.dispatchEvent(
new Event('play', {
bubbles: false,
}),
);
// Regression test: ensure React tree delegation still works
// even if the actual DOM element did not have a handler.
expect(handleVideoPlayDelegated).toHaveBeenCalledTimes(1);
} finally {
document.body.removeChild(container);
}
});
});
109 changes: 95 additions & 14 deletions packages/react-dom/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,18 @@ import {
enableDeprecatedFlareAPI,
enableTrustedTypesIntegration,
} from 'shared/ReactFeatureFlags';
import {listenToReactEvent} from '../events/DOMModernPluginEventSystem';
import {
listenToReactEvent,
mediaEventTypes,
listenToNonDelegatedEvent,
} from '../events/DOMModernPluginEventSystem';
import {getEventListenerMap} from './ReactDOMComponentTree';
import {
TOP_LOAD,
TOP_ERROR,
TOP_TOGGLE,
TOP_INVALID,
} from '../events/DOMTopLevelEventTypes';

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

function getOwnerDocumentFromRootContainer(
Expand Down Expand Up @@ -364,7 +379,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 @@ -527,32 +542,50 @@ export function setInitialProperties(
case 'iframe':
case 'object':
case 'embed':
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the load event.
listenToNonDelegatedEvent(TOP_LOAD, domElement);
props = rawProps;
break;
case 'video':
case 'audio':
// We listen to these events in case to ensure emulated bubble
// listeners still fire for all the media events.
for (let i = 0; i < mediaEventTypes.length; i++) {
listenToNonDelegatedEvent(mediaEventTypes[i], domElement);
}
props = rawProps;
break;
case 'source':
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the error event.
listenToNonDelegatedEvent(TOP_ERROR, domElement);
props = rawProps;
break;
case 'img':
case 'image':
case 'link':
props = rawProps;
break;
case 'form':
// We listen to these events in case to ensure emulated bubble
// listeners still fire for error and load events.
listenToNonDelegatedEvent(TOP_ERROR, domElement);
listenToNonDelegatedEvent(TOP_LOAD, domElement);
props = rawProps;
break;
case 'details':
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the toggle event.
listenToNonDelegatedEvent(TOP_TOGGLE, domElement);
props = rawProps;
break;
case 'input':
ReactDOMInputInitWrapperState(domElement, rawProps);
props = ReactDOMInputGetHostProps(domElement, rawProps);
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent(TOP_INVALID, domElement);
// 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 @@ -561,16 +594,22 @@ export function setInitialProperties(
case 'select':
ReactDOMSelectInitWrapperState(domElement, rawProps);
props = ReactDOMSelectGetHostProps(domElement, rawProps);
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent(TOP_INVALID, domElement);
// 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);
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent(TOP_INVALID, domElement);
// 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 @@ -790,7 +829,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 @@ -900,26 +939,68 @@ export function diffHydratedProperties(

// TODO: Make sure that we check isMounted before firing any of these events.
switch (tag) {
case 'iframe':
case 'object':
case 'embed':
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the load event.
listenToNonDelegatedEvent(TOP_LOAD, domElement);
break;
case 'video':
case 'audio':
// We listen to these events in case to ensure emulated bubble
// listeners still fire for all the media events.
for (let i = 0; i < mediaEventTypes.length; i++) {
listenToNonDelegatedEvent(mediaEventTypes[i], domElement);
}
break;
case 'source':
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the error event.
listenToNonDelegatedEvent(TOP_ERROR, domElement);
break;
case 'img':
case 'image':
case 'link':
// We listen to these events in case to ensure emulated bubble
// listeners still fire for error and load events.
listenToNonDelegatedEvent(TOP_ERROR, domElement);
listenToNonDelegatedEvent(TOP_LOAD, domElement);
break;
case 'details':
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the toggle event.
listenToNonDelegatedEvent(TOP_TOGGLE, domElement);
break;
case 'input':
ReactDOMInputInitWrapperState(domElement, rawProps);
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent(TOP_INVALID, domElement);
// 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);
break;
case 'select':
ReactDOMSelectInitWrapperState(domElement, rawProps);
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent(TOP_INVALID, domElement);
// 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);
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent(TOP_INVALID, domElement);
// 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 @@ -986,7 +1067,7 @@ export function diffHydratedProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
ensureListeningTo(rootContainerElement, propKey);
ensureListeningTo(rootContainerElement, propKey, domElement);
}
} else if (
__DEV__ &&
Expand Down
19 changes: 10 additions & 9 deletions packages/react-dom/src/client/ReactDOMEventHandle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
getClosestInstanceFromNode,
getEventHandlerListeners,
setEventHandlerListeners,
getEventListenerMap,
getFiberFromScopeInstance,
} from './ReactDOMComponentTree';
import {ELEMENT_NODE, COMMENT_NODE} from '../shared/HTMLNodeType';
Expand Down Expand Up @@ -87,6 +86,7 @@ function registerEventOnNearestTargetContainer(
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.
Expand All @@ -101,13 +101,11 @@ function registerEventOnNearestTargetContainer(
if (targetContainer.nodeType === COMMENT_NODE) {
targetContainer = ((targetContainer.parentNode: any): Element);
}
const listenerMap = getEventListenerMap(targetContainer);
listenToNativeEvent(
topLevelType,
targetContainer,
listenerMap,
PLUGIN_EVENT_SYSTEM,
isCapturePhaseListener,
targetContainer,
targetElement,
isPassiveListener,
listenerPriority,
);
Expand Down Expand Up @@ -138,6 +136,7 @@ function registerReactDOMEvent(
isPassiveListener,
listenerPriority,
isCapturePhaseListener,
targetElement,
);
} else if (enableScopeAPI && isReactScope(target)) {
const scopeTarget = ((target: any): ReactScopeInstance);
Expand All @@ -152,18 +151,20 @@ function registerReactDOMEvent(
isPassiveListener,
listenerPriority,
isCapturePhaseListener,
null,
);
} else if (isValidEventTarget(target)) {
const eventTarget = ((target: any): EventTarget);
const listenerMap = getEventListenerMap(eventTarget);
// These are valid event targets, but they are also
// non-managed React nodes.
listenToNativeEvent(
topLevelType,
eventTarget,
listenerMap,
PLUGIN_EVENT_SYSTEM | IS_EVENT_HANDLE_NON_MANAGED_NODE,
isCapturePhaseListener,
eventTarget,
null,
isPassiveListener,
listenerPriority,
PLUGIN_EVENT_SYSTEM | IS_EVENT_HANDLE_NON_MANAGED_NODE,
);
} else {
invariant(
Expand Down
Loading

0 comments on commit 352b88b

Please sign in to comment.