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.

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

Attach Listeners Eagerly to Roots and Portal Containers #19659

Merged
merged 4 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 34 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,18 +398,49 @@ describe('ReactDOMEventListener', () => {
const originalDocAddEventListener = document.addEventListener;
const originalRootAddEventListener = container.addEventListener;
document.addEventListener = function(type) {
switch (type) {
case 'selectionchange':
break;
default:
throw new Error(
`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.
container.addEventListener = function(type, fn, options) {
if (options && (options === true || options.capture)) {
return;
}
switch (type) {
case 'abort':
case 'canplay':
case 'canplaythrough':
case 'durationchange':
case 'emptied':
case 'encrypted':
case 'ended':
case 'error':
case 'loadeddata':
case 'loadedmetadata':
case 'loadstart':
case 'pause':
case 'play':
case 'playing':
case 'progress':
case 'ratechange':
case 'seeked':
case 'seeking':
case 'stalled':
case 'suspend':
case 'timeupdate':
case 'volumechange':
case 'waiting':
throw new Error(
`Did not expect to add a root-level listener for the "${type}" event.`,
);
default:
break;
}
};

try {
Expand Down
19 changes: 19 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,25 @@ describe('ReactDOMFiber', () => {
expect(ops).toEqual([]);
});

// @gate enableEagerRootListeners
it('listens to events that do not exist in the Portal subtree', () => {
const onClick = jest.fn();

const ref = React.createRef();
ReactDOM.render(
<div onClick={onClick}>
{ReactDOM.createPortal(<button ref={ref}>click</button>, document.body)}
</div>,
container,
);
const event = new MouseEvent('click', {
bubbles: true,
});
ref.current.dispatchEvent(event);

expect(onClick).toHaveBeenCalledTimes(1);
});

it('should throw on bad createPortal argument', () => {
expect(() => {
ReactDOM.createPortal(<div>portal</div>, null);
Expand Down
29 changes: 28 additions & 1 deletion packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ import {validateProperties as validateInputProperties} from '../shared/ReactDOMN
import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook';
import {REACT_OPAQUE_ID_TYPE} from 'shared/ReactSymbols';

import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';
import {
enableTrustedTypesIntegration,
enableEagerRootListeners,
} from 'shared/ReactFeatureFlags';
import {
listenToReactEvent,
mediaEventTypes,
Expand Down Expand Up @@ -260,6 +263,7 @@ export function ensureListeningTo(
reactPropEvent: string,
targetElement: Element | null,
): void {
if (!enableEagerRootListeners) {
// If we have a comment node, then use the parent node,
// which should be an element.
const rootContainerElement =
Expand All @@ -285,6 +289,7 @@ export function ensureListeningTo(
targetElement,
);
}
}

function getOwnerDocumentFromRootContainer(
rootContainerElement: Element | Document,
Expand Down Expand Up @@ -364,7 +369,11 @@ function setInitialDOMProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
if (!enableEagerRootListeners) {
ensureListeningTo(rootContainerElement, propKey, domElement);
} else if (propKey === 'onScroll') {
listenToNonDelegatedEvent('scroll', domElement);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can't avoid for scroll since it only bubbles to document.

}
}
} else if (nextProp != null) {
setValueForProperty(domElement, propKey, nextProp, isCustomComponentTag);
Expand Down Expand Up @@ -573,9 +582,11 @@ export function setInitialProperties(
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent('invalid', domElement);
if (!enableEagerRootListeners) {
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange', domElement);
}
break;
case 'option':
ReactDOMOptionValidateProps(domElement, rawProps);
Expand All @@ -587,19 +598,23 @@ export function setInitialProperties(
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent('invalid', domElement);
if (!enableEagerRootListeners) {
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
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('invalid', domElement);
if (!enableEagerRootListeners) {
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange', domElement);
}
break;
default:
props = rawProps;
Expand Down Expand Up @@ -817,7 +832,11 @@ export function diffProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
if (!enableEagerRootListeners) {
ensureListeningTo(rootContainerElement, propKey, domElement);
} else if (propKey === 'onScroll') {
listenToNonDelegatedEvent('scroll', domElement);
}
}
if (!updatePayload && lastProp !== nextProp) {
// This is a special case. If any listener updates we need to ensure
Expand Down Expand Up @@ -969,9 +988,11 @@ export function diffHydratedProperties(
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent('invalid', domElement);
if (!enableEagerRootListeners) {
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange', domElement);
}
break;
case 'option':
ReactDOMOptionValidateProps(domElement, rawProps);
Expand All @@ -981,18 +1002,22 @@ export function diffHydratedProperties(
// We listen to this event in case to ensure emulated bubble
// listeners still fire for the invalid event.
listenToNonDelegatedEvent('invalid', domElement);
if (!enableEagerRootListeners) {
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
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('invalid', domElement);
if (!enableEagerRootListeners) {
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
ensureListeningTo(rootContainerElement, 'onChange', domElement);
}
break;
}

Expand Down Expand Up @@ -1059,8 +1084,10 @@ export function diffHydratedProperties(
if (__DEV__ && typeof nextProp !== 'function') {
warnForInvalidEventListener(propKey, nextProp);
}
if (!enableEagerRootListeners) {
ensureListeningTo(rootContainerElement, propKey, domElement);
}
}
} else if (
__DEV__ &&
// Convince Flow we've calculated it (it's DEV-only in this method.)
Expand Down
22 changes: 22 additions & 0 deletions packages/react-dom/src/client/ReactDOMEventHandle.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
} from '../shared/ReactDOMTypes';

import {getEventPriorityForListenerSystem} from '../events/DOMEventProperties';
import {allNativeEvents} from '../events/EventRegistry';
import {
getClosestInstanceFromNode,
getEventHandlerListeners,
Expand All @@ -33,6 +34,7 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../events/EventSystemFlags';
import {
enableScopeAPI,
enableCreateEventHandleAPI,
enableEagerRootListeners,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';

Expand Down Expand Up @@ -178,6 +180,26 @@ export function createEventHandle(
): ReactDOMEventHandle {
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' ||
domEventName === 'afterblur',
'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;
Expand Down
10 changes: 9 additions & 1 deletion packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,13 @@ import {
enableFundamentalAPI,
enableCreateEventHandleAPI,
enableScopeAPI,
enableEagerRootListeners,
} from 'shared/ReactFeatureFlags';
import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
import {listenToReactEvent} from '../events/DOMPluginEventSystem';
import {
listenToReactEvent,
listenToAllSupportedEvents,
} from '../events/DOMPluginEventSystem';

export type Type = string;
export type Props = {
Expand Down Expand Up @@ -1069,8 +1073,12 @@ export function makeOpaqueHydratingObject(
}

export function preparePortalMount(portalInstance: Instance): void {
if (enableEagerRootListeners) {
listenToAllSupportedEvents(portalInstance);
} else {
listenToReactEvent('onMouseEnter', portalInstance, null);
}
}

export function prepareScopeUpdate(
scopeInstance: ReactScopeInstance,
Expand Down
12 changes: 11 additions & 1 deletion packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
markContainerAsRoot,
unmarkContainerAsRoot,
} from './ReactDOMComponentTree';
import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';
import {eagerlyTrapReplayableEvents} from '../events/ReactDOMEventReplaying';
import {
ELEMENT_NODE,
Expand All @@ -51,6 +52,7 @@ import {
registerMutableSourceForHydration,
} from 'react-reconciler/src/ReactFiberReconciler';
import invariant from 'shared/invariant';
import {enableEagerRootListeners} from 'shared/ReactFeatureFlags';
import {
BlockingRoot,
ConcurrentRoot,
Expand Down Expand Up @@ -133,9 +135,16 @@ function createRootImpl(
markContainerAsRoot(root.current, container);
const containerNodeType = container.nodeType;

if (enableEagerRootListeners) {
const rootContainerElement =
container.nodeType === COMMENT_NODE ? container.parentNode : container;
listenToAllSupportedEvents(rootContainerElement);
} else {
if (hydrate && tag !== LegacyRoot) {
const doc =
containerNodeType === DOCUMENT_NODE ? container : container.ownerDocument;
containerNodeType === DOCUMENT_NODE
? container
: container.ownerDocument;
// We need to cast this because Flow doesn't work
// with the hoisted containerNodeType. If we inline
// it, then Flow doesn't complain. We intentionally
Expand All @@ -147,6 +156,7 @@ function createRootImpl(
) {
ensureListeningTo(container, 'onMouseEnter', null);
}
}

if (mutableSources) {
for (let i = 0; i < mutableSources.length; i++) {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-dom/src/events/DOMEventProperties.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,9 @@ export function getEventPriorityForListenerSystem(
}
if (__DEV__) {
console.warn(
'The event "type" provided to createEventHandle() does not have a known priority type.' +
'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.',
type,
);
}
return ContinuousEvent;
Expand Down
Loading