Skip to content

Commit 848bb24

Browse files
gaearonkoba04
gaearon
and
koba04
authored
Attach Listeners Eagerly to Roots and Portal Containers (#19659)
* Failing test for #19608 * Attach Listeners Eagerly to Roots and Portal Containers * Forbid createEventHandle with custom events We can't support this without adding more complexity. It's not clear that this is even desirable, as none of our existing use cases need custom events. This API primarily exists as a deprecation strategy for Flare, so I don't think it is important to expand its support beyond what Flare replacement code currently needs. We can later revisit it with a better understanding of the eager/lazy tradeoff but for now let's remove the inconsistency. * Reduce risk by changing condition only under the flag Co-authored-by: koba04 <[email protected]>
1 parent 8c9fc4e commit 848bb24

25 files changed

+527
-233
lines changed

‎packages/react-dom/src/__tests__/ReactDOMEventListener-test.js

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -398,18 +398,49 @@ describe('ReactDOMEventListener', () => {
398398
const originalDocAddEventListener = document.addEventListener;
399399
const originalRootAddEventListener = container.addEventListener;
400400
document.addEventListener = function(type) {
401-
throw new Error(
402-
`Did not expect to add a document-level listener for the "${type}" event.`,
403-
);
401+
switch (type) {
402+
case 'selectionchange':
403+
break;
404+
default:
405+
throw new Error(
406+
`Did not expect to add a document-level listener for the "${type}" event.`,
407+
);
408+
}
404409
};
405-
container.addEventListener = function(type) {
406-
if (type === 'mouseout' || type === 'mouseover') {
407-
// We currently listen to it unconditionally.
410+
container.addEventListener = function(type, fn, options) {
411+
if (options && (options === true || options.capture)) {
408412
return;
409413
}
410-
throw new Error(
411-
`Did not expect to add a root-level listener for the "${type}" event.`,
412-
);
414+
switch (type) {
415+
case 'abort':
416+
case 'canplay':
417+
case 'canplaythrough':
418+
case 'durationchange':
419+
case 'emptied':
420+
case 'encrypted':
421+
case 'ended':
422+
case 'error':
423+
case 'loadeddata':
424+
case 'loadedmetadata':
425+
case 'loadstart':
426+
case 'pause':
427+
case 'play':
428+
case 'playing':
429+
case 'progress':
430+
case 'ratechange':
431+
case 'seeked':
432+
case 'seeking':
433+
case 'stalled':
434+
case 'suspend':
435+
case 'timeupdate':
436+
case 'volumechange':
437+
case 'waiting':
438+
throw new Error(
439+
`Did not expect to add a root-level listener for the "${type}" event.`,
440+
);
441+
default:
442+
break;
443+
}
413444
};
414445

415446
try {

‎packages/react-dom/src/__tests__/ReactDOMFiber-test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,25 @@ describe('ReactDOMFiber', () => {
10401040
expect(ops).toEqual([]);
10411041
});
10421042

1043+
// @gate enableEagerRootListeners
1044+
it('listens to events that do not exist in the Portal subtree', () => {
1045+
const onClick = jest.fn();
1046+
1047+
const ref = React.createRef();
1048+
ReactDOM.render(
1049+
<div onClick={onClick}>
1050+
{ReactDOM.createPortal(<button ref={ref}>click</button>, document.body)}
1051+
</div>,
1052+
container,
1053+
);
1054+
const event = new MouseEvent('click', {
1055+
bubbles: true,
1056+
});
1057+
ref.current.dispatchEvent(event);
1058+
1059+
expect(onClick).toHaveBeenCalledTimes(1);
1060+
});
1061+
10431062
it('should throw on bad createPortal argument', () => {
10441063
expect(() => {
10451064
ReactDOM.createPortal(<div>portal</div>, null);

‎packages/react-dom/src/client/ReactDOMComponent.js

Lines changed: 71 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ import {validateProperties as validateInputProperties} from '../shared/ReactDOMN
7474
import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook';
7575
import {REACT_OPAQUE_ID_TYPE} from 'shared/ReactSymbols';
7676

77-
import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';
77+
import {
78+
enableTrustedTypesIntegration,
79+
enableEagerRootListeners,
80+
} from 'shared/ReactFeatureFlags';
7881
import {
7982
listenToReactEvent,
8083
mediaEventTypes,
@@ -260,30 +263,32 @@ export function ensureListeningTo(
260263
reactPropEvent: string,
261264
targetElement: Element | null,
262265
): void {
263-
// If we have a comment node, then use the parent node,
264-
// which should be an element.
265-
const rootContainerElement =
266-
rootContainerInstance.nodeType === COMMENT_NODE
267-
? rootContainerInstance.parentNode
268-
: rootContainerInstance;
269-
if (__DEV__) {
270-
if (
271-
rootContainerElement == null ||
272-
(rootContainerElement.nodeType !== ELEMENT_NODE &&
273-
// This is to support rendering into a ShadowRoot:
274-
rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE)
275-
) {
276-
console.error(
277-
'ensureListeningTo(): received a container that was not an element node. ' +
278-
'This is likely a bug in React. Please file an issue.',
279-
);
266+
if (!enableEagerRootListeners) {
267+
// If we have a comment node, then use the parent node,
268+
// which should be an element.
269+
const rootContainerElement =
270+
rootContainerInstance.nodeType === COMMENT_NODE
271+
? rootContainerInstance.parentNode
272+
: rootContainerInstance;
273+
if (__DEV__) {
274+
if (
275+
rootContainerElement == null ||
276+
(rootContainerElement.nodeType !== ELEMENT_NODE &&
277+
// This is to support rendering into a ShadowRoot:
278+
rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE)
279+
) {
280+
console.error(
281+
'ensureListeningTo(): received a container that was not an element node. ' +
282+
'This is likely a bug in React. Please file an issue.',
283+
);
284+
}
280285
}
286+
listenToReactEvent(
287+
reactPropEvent,
288+
((rootContainerElement: any): Element),
289+
targetElement,
290+
);
281291
}
282-
listenToReactEvent(
283-
reactPropEvent,
284-
((rootContainerElement: any): Element),
285-
targetElement,
286-
);
287292
}
288293

289294
function getOwnerDocumentFromRootContainer(
@@ -364,7 +369,11 @@ function setInitialDOMProperties(
364369
if (__DEV__ && typeof nextProp !== 'function') {
365370
warnForInvalidEventListener(propKey, nextProp);
366371
}
367-
ensureListeningTo(rootContainerElement, propKey, domElement);
372+
if (!enableEagerRootListeners) {
373+
ensureListeningTo(rootContainerElement, propKey, domElement);
374+
} else if (propKey === 'onScroll') {
375+
listenToNonDelegatedEvent('scroll', domElement);
376+
}
368377
}
369378
} else if (nextProp != null) {
370379
setValueForProperty(domElement, propKey, nextProp, isCustomComponentTag);
@@ -573,9 +582,11 @@ export function setInitialProperties(
573582
// We listen to this event in case to ensure emulated bubble
574583
// listeners still fire for the invalid event.
575584
listenToNonDelegatedEvent('invalid', domElement);
576-
// For controlled components we always need to ensure we're listening
577-
// to onChange. Even if there is no listener.
578-
ensureListeningTo(rootContainerElement, 'onChange', domElement);
585+
if (!enableEagerRootListeners) {
586+
// For controlled components we always need to ensure we're listening
587+
// to onChange. Even if there is no listener.
588+
ensureListeningTo(rootContainerElement, 'onChange', domElement);
589+
}
579590
break;
580591
case 'option':
581592
ReactDOMOptionValidateProps(domElement, rawProps);
@@ -587,19 +598,23 @@ export function setInitialProperties(
587598
// We listen to this event in case to ensure emulated bubble
588599
// listeners still fire for the invalid event.
589600
listenToNonDelegatedEvent('invalid', domElement);
590-
// For controlled components we always need to ensure we're listening
591-
// to onChange. Even if there is no listener.
592-
ensureListeningTo(rootContainerElement, 'onChange', domElement);
601+
if (!enableEagerRootListeners) {
602+
// For controlled components we always need to ensure we're listening
603+
// to onChange. Even if there is no listener.
604+
ensureListeningTo(rootContainerElement, 'onChange', domElement);
605+
}
593606
break;
594607
case 'textarea':
595608
ReactDOMTextareaInitWrapperState(domElement, rawProps);
596609
props = ReactDOMTextareaGetHostProps(domElement, rawProps);
597610
// We listen to this event in case to ensure emulated bubble
598611
// listeners still fire for the invalid event.
599612
listenToNonDelegatedEvent('invalid', domElement);
600-
// For controlled components we always need to ensure we're listening
601-
// to onChange. Even if there is no listener.
602-
ensureListeningTo(rootContainerElement, 'onChange', domElement);
613+
if (!enableEagerRootListeners) {
614+
// For controlled components we always need to ensure we're listening
615+
// to onChange. Even if there is no listener.
616+
ensureListeningTo(rootContainerElement, 'onChange', domElement);
617+
}
603618
break;
604619
default:
605620
props = rawProps;
@@ -817,7 +832,11 @@ export function diffProperties(
817832
if (__DEV__ && typeof nextProp !== 'function') {
818833
warnForInvalidEventListener(propKey, nextProp);
819834
}
820-
ensureListeningTo(rootContainerElement, propKey, domElement);
835+
if (!enableEagerRootListeners) {
836+
ensureListeningTo(rootContainerElement, propKey, domElement);
837+
} else if (propKey === 'onScroll') {
838+
listenToNonDelegatedEvent('scroll', domElement);
839+
}
821840
}
822841
if (!updatePayload && lastProp !== nextProp) {
823842
// This is a special case. If any listener updates we need to ensure
@@ -969,9 +988,11 @@ export function diffHydratedProperties(
969988
// We listen to this event in case to ensure emulated bubble
970989
// listeners still fire for the invalid event.
971990
listenToNonDelegatedEvent('invalid', domElement);
972-
// For controlled components we always need to ensure we're listening
973-
// to onChange. Even if there is no listener.
974-
ensureListeningTo(rootContainerElement, 'onChange', domElement);
991+
if (!enableEagerRootListeners) {
992+
// For controlled components we always need to ensure we're listening
993+
// to onChange. Even if there is no listener.
994+
ensureListeningTo(rootContainerElement, 'onChange', domElement);
995+
}
975996
break;
976997
case 'option':
977998
ReactDOMOptionValidateProps(domElement, rawProps);
@@ -981,18 +1002,22 @@ export function diffHydratedProperties(
9811002
// We listen to this event in case to ensure emulated bubble
9821003
// listeners still fire for the invalid event.
9831004
listenToNonDelegatedEvent('invalid', domElement);
984-
// For controlled components we always need to ensure we're listening
985-
// to onChange. Even if there is no listener.
986-
ensureListeningTo(rootContainerElement, 'onChange', domElement);
1005+
if (!enableEagerRootListeners) {
1006+
// For controlled components we always need to ensure we're listening
1007+
// to onChange. Even if there is no listener.
1008+
ensureListeningTo(rootContainerElement, 'onChange', domElement);
1009+
}
9871010
break;
9881011
case 'textarea':
9891012
ReactDOMTextareaInitWrapperState(domElement, rawProps);
9901013
// We listen to this event in case to ensure emulated bubble
9911014
// listeners still fire for the invalid event.
9921015
listenToNonDelegatedEvent('invalid', domElement);
993-
// For controlled components we always need to ensure we're listening
994-
// to onChange. Even if there is no listener.
995-
ensureListeningTo(rootContainerElement, 'onChange', domElement);
1016+
if (!enableEagerRootListeners) {
1017+
// For controlled components we always need to ensure we're listening
1018+
// to onChange. Even if there is no listener.
1019+
ensureListeningTo(rootContainerElement, 'onChange', domElement);
1020+
}
9961021
break;
9971022
}
9981023

@@ -1059,7 +1084,9 @@ export function diffHydratedProperties(
10591084
if (__DEV__ && typeof nextProp !== 'function') {
10601085
warnForInvalidEventListener(propKey, nextProp);
10611086
}
1062-
ensureListeningTo(rootContainerElement, propKey, domElement);
1087+
if (!enableEagerRootListeners) {
1088+
ensureListeningTo(rootContainerElement, propKey, domElement);
1089+
}
10631090
}
10641091
} else if (
10651092
__DEV__ &&

‎packages/react-dom/src/client/ReactDOMEventHandle.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {
1515
} from '../shared/ReactDOMTypes';
1616

1717
import {getEventPriorityForListenerSystem} from '../events/DOMEventProperties';
18+
import {allNativeEvents} from '../events/EventRegistry';
1819
import {
1920
getClosestInstanceFromNode,
2021
getEventHandlerListeners,
@@ -35,6 +36,7 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../events/EventSystemFlags';
3536
import {
3637
enableScopeAPI,
3738
enableCreateEventHandleAPI,
39+
enableEagerRootListeners,
3840
} from 'shared/ReactFeatureFlags';
3941
import invariant from 'shared/invariant';
4042

@@ -178,6 +180,26 @@ export function createEventHandle(
178180
): ReactDOMEventHandle {
179181
if (enableCreateEventHandleAPI) {
180182
const domEventName = ((type: any): DOMEventName);
183+
184+
if (enableEagerRootListeners) {
185+
// We cannot support arbitrary native events with eager root listeners
186+
// because the eager strategy relies on knowing the whole list ahead of time.
187+
// If we wanted to support this, we'd have to add code to keep track
188+
// (or search) for all portal and root containers, and lazily add listeners
189+
// to them whenever we see a previously unknown event. This seems like a lot
190+
// of complexity for something we don't even have a particular use case for.
191+
// Unfortunately, the downside of this invariant is that *removing* a native
192+
// event from the list of known events has now become a breaking change for
193+
// any code relying on the createEventHandle API.
194+
invariant(
195+
allNativeEvents.has(domEventName) ||
196+
domEventName === 'beforeblur' ||
197+
domEventName === 'afterblur',
198+
'Cannot call unstable_createEventHandle with "%s", as it is not an event known to React.',
199+
domEventName,
200+
);
201+
}
202+
181203
let isCapturePhaseListener = false;
182204
let isPassiveListener = undefined; // Undefined means to use the browser default
183205
let listenerPriority;

‎packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,13 @@ import {
6767
enableFundamentalAPI,
6868
enableCreateEventHandleAPI,
6969
enableScopeAPI,
70+
enableEagerRootListeners,
7071
} from 'shared/ReactFeatureFlags';
7172
import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
72-
import {listenToReactEvent} from '../events/DOMPluginEventSystem';
73+
import {
74+
listenToReactEvent,
75+
listenToAllSupportedEvents,
76+
} from '../events/DOMPluginEventSystem';
7377

7478
export type Type = string;
7579
export type Props = {
@@ -1069,7 +1073,11 @@ export function makeOpaqueHydratingObject(
10691073
}
10701074

10711075
export function preparePortalMount(portalInstance: Instance): void {
1072-
listenToReactEvent('onMouseEnter', portalInstance, null);
1076+
if (enableEagerRootListeners) {
1077+
listenToAllSupportedEvents(portalInstance);
1078+
} else {
1079+
listenToReactEvent('onMouseEnter', portalInstance, null);
1080+
}
10731081
}
10741082

10751083
export function prepareScopeUpdate(

0 commit comments

Comments
 (0)