From 848bb2426e44606e0a55dfe44c7b3ece33772485 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 24 Aug 2020 16:50:20 +0100 Subject: [PATCH] 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 --- .../__tests__/ReactDOMEventListener-test.js | 49 +++- .../src/__tests__/ReactDOMFiber-test.js | 19 ++ .../react-dom/src/client/ReactDOMComponent.js | 115 ++++++---- .../src/client/ReactDOMEventHandle.js | 22 ++ .../src/client/ReactDOMHostConfig.js | 12 +- packages/react-dom/src/client/ReactDOMRoot.js | 36 +-- .../src/events/DOMEventProperties.js | 3 +- .../src/events/DOMPluginEventSystem.js | 125 ++++++---- .../react-dom/src/events/EventRegistry.js | 14 +- .../src/events/ReactDOMEventListener.js | 78 ++++--- .../src/events/ReactDOMEventReplaying.js | 29 ++- .../DOMPluginEventSystem-test.internal.js | 214 ++++++++++++------ .../src/events/plugins/SelectEventPlugin.js | 29 +-- .../__tests__/SimpleEventPlugin-test.js | 13 +- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/error-codes/codes.json | 3 +- 25 files changed, 533 insertions(+), 239 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 5cb207409d..bced36b0ff 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -398,18 +398,49 @@ describe('ReactDOMEventListener', () => { const originalDocAddEventListener = document.addEventListener; const originalRootAddEventListener = container.addEventListener; document.addEventListener = function(type) { - throw new Error( - `Did not expect to add a document-level listener for the "${type}" event.`, - ); + 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; } - throw new Error( - `Did not expect to add a root-level listener for the "${type}" event.`, - ); + 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 { diff --git a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js index b05b4c1f96..a91c2fac41 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js @@ -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( +
+ {ReactDOM.createPortal(, document.body)} +
, + 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(
portal
, null); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index d3e72d2a2d..48b979af64 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -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, @@ -260,30 +263,32 @@ export function ensureListeningTo( reactPropEvent: string, targetElement: Element | null, ): void { - // If we have a comment node, then use the parent node, - // which should be an element. - const rootContainerElement = - rootContainerInstance.nodeType === COMMENT_NODE - ? rootContainerInstance.parentNode - : rootContainerInstance; - if (__DEV__) { - if ( - rootContainerElement == null || - (rootContainerElement.nodeType !== ELEMENT_NODE && - // This is to support rendering into a ShadowRoot: - rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE) - ) { - console.error( - 'ensureListeningTo(): received a container that was not an element node. ' + - 'This is likely a bug in React. Please file an issue.', - ); + if (!enableEagerRootListeners) { + // If we have a comment node, then use the parent node, + // which should be an element. + const rootContainerElement = + rootContainerInstance.nodeType === COMMENT_NODE + ? rootContainerInstance.parentNode + : rootContainerInstance; + if (__DEV__) { + if ( + rootContainerElement == null || + (rootContainerElement.nodeType !== ELEMENT_NODE && + // This is to support rendering into a ShadowRoot: + rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE) + ) { + console.error( + 'ensureListeningTo(): received a container that was not an element node. ' + + 'This is likely a bug in React. Please file an issue.', + ); + } } + listenToReactEvent( + reactPropEvent, + ((rootContainerElement: any): Element), + targetElement, + ); } - listenToReactEvent( - reactPropEvent, - ((rootContainerElement: any): Element), - targetElement, - ); } function getOwnerDocumentFromRootContainer( @@ -364,7 +369,11 @@ function setInitialDOMProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - ensureListeningTo(rootContainerElement, propKey, domElement); + if (!enableEagerRootListeners) { + ensureListeningTo(rootContainerElement, propKey, domElement); + } else if (propKey === 'onScroll') { + listenToNonDelegatedEvent('scroll', domElement); + } } } else if (nextProp != null) { setValueForProperty(domElement, propKey, nextProp, isCustomComponentTag); @@ -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); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', 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); @@ -587,9 +598,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); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', 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); @@ -597,9 +610,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); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', 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; @@ -817,7 +832,11 @@ export function diffProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - ensureListeningTo(rootContainerElement, propKey, domElement); + 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 @@ -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); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', 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); @@ -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); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', 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); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', 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; } @@ -1059,7 +1084,9 @@ export function diffHydratedProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - ensureListeningTo(rootContainerElement, propKey, domElement); + if (!enableEagerRootListeners) { + ensureListeningTo(rootContainerElement, propKey, domElement); + } } } else if ( __DEV__ && diff --git a/packages/react-dom/src/client/ReactDOMEventHandle.js b/packages/react-dom/src/client/ReactDOMEventHandle.js index 303b083313..52edc8aaf7 100644 --- a/packages/react-dom/src/client/ReactDOMEventHandle.js +++ b/packages/react-dom/src/client/ReactDOMEventHandle.js @@ -15,6 +15,7 @@ import type { } from '../shared/ReactDOMTypes'; import {getEventPriorityForListenerSystem} from '../events/DOMEventProperties'; +import {allNativeEvents} from '../events/EventRegistry'; import { getClosestInstanceFromNode, getEventHandlerListeners, @@ -35,6 +36,7 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../events/EventSystemFlags'; import { enableScopeAPI, enableCreateEventHandleAPI, + enableEagerRootListeners, } from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; @@ -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; diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 22d38dea04..e5ce50add7 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -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 = { @@ -1069,7 +1073,11 @@ export function makeOpaqueHydratingObject( } export function preparePortalMount(portalInstance: Instance): void { - listenToReactEvent('onMouseEnter', portalInstance, null); + if (enableEagerRootListeners) { + listenToAllSupportedEvents(portalInstance); + } else { + listenToReactEvent('onMouseEnter', portalInstance, null); + } } export function prepareScopeUpdate( diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index d9e446749c..11ec850a4a 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -35,6 +35,7 @@ import { markContainerAsRoot, unmarkContainerAsRoot, } from './ReactDOMComponentTree'; +import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem'; import {eagerlyTrapReplayableEvents} from '../events/ReactDOMEventReplaying'; import { ELEMENT_NODE, @@ -51,6 +52,7 @@ import { registerMutableSourceForHydration, } from 'react-reconciler/src/ReactFiberReconciler'; import invariant from 'shared/invariant'; +import {enableEagerRootListeners} from 'shared/ReactFeatureFlags'; import { BlockingRoot, ConcurrentRoot, @@ -133,19 +135,27 @@ function createRootImpl( markContainerAsRoot(root.current, container); const containerNodeType = container.nodeType; - if (hydrate && tag !== LegacyRoot) { - const doc = - 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 - // hoist it to reduce code-size. - eagerlyTrapReplayableEvents(container, ((doc: any): Document)); - } else if ( - containerNodeType !== DOCUMENT_FRAGMENT_NODE && - containerNodeType !== DOCUMENT_NODE - ) { - ensureListeningTo(container, 'onMouseEnter', null); + 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; + // 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 + // hoist it to reduce code-size. + eagerlyTrapReplayableEvents(container, ((doc: any): Document)); + } else if ( + containerNodeType !== DOCUMENT_FRAGMENT_NODE && + containerNodeType !== DOCUMENT_NODE + ) { + ensureListeningTo(container, 'onMouseEnter', null); + } } if (mutableSources) { diff --git a/packages/react-dom/src/events/DOMEventProperties.js b/packages/react-dom/src/events/DOMEventProperties.js index 342e653bd4..8c5090ecae 100644 --- a/packages/react-dom/src/events/DOMEventProperties.js +++ b/packages/react-dom/src/events/DOMEventProperties.js @@ -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; diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 6a216ace12..97a37ace2e 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -23,7 +23,7 @@ import type {ElementListenerMapEntry} from '../client/ReactDOMComponentTree'; import type {EventPriority} from 'shared/ReactTypes'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; -import {registrationNameDependencies} from './EventRegistry'; +import {registrationNameDependencies, allNativeEvents} from './EventRegistry'; import { IS_CAPTURE_PHASE, IS_EVENT_HANDLE_NON_MANAGED_NODE, @@ -54,11 +54,13 @@ import { enableCreateEventHandleAPI, enableScopeAPI, enablePassiveEventIntervention, + enableEagerRootListeners, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, rethrowCaughtError, } from 'shared/ReactErrorUtils'; +import {DOCUMENT_NODE} from '../shared/HTMLNodeType'; import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener'; import { removeEventListener, @@ -327,6 +329,41 @@ export function listenToNonDelegatedEvent( } } +const listeningMarker = + '_reactListening' + + Math.random() + .toString(36) + .slice(2); + +export function listenToAllSupportedEvents(rootContainerElement: EventTarget) { + if (enableEagerRootListeners) { + if ((rootContainerElement: any)[listeningMarker]) { + // Performance optimization: don't iterate through events + // for the same portal container or root node more than once. + // TODO: once we remove the flag, we may be able to also + // remove some of the bookkeeping maps used for laziness. + return; + } + (rootContainerElement: any)[listeningMarker] = true; + allNativeEvents.forEach(domEventName => { + if (!nonDelegatedEvents.has(domEventName)) { + listenToNativeEvent( + domEventName, + false, + ((rootContainerElement: any): Element), + null, + ); + } + listenToNativeEvent( + domEventName, + true, + ((rootContainerElement: any): Element), + null, + ); + }); + } +} + export function listenToNativeEvent( domEventName: DOMEventName, isCapturePhaseListener: boolean, @@ -337,10 +374,14 @@ export function listenToNativeEvent( eventSystemFlags?: EventSystemFlags = 0, ): void { let target = rootContainerElement; + // selectionchange needs to be attached to the document // otherwise it won't capture incoming events that are only // triggered on the document directly. - if (domEventName === 'selectionchange') { + if ( + domEventName === 'selectionchange' && + (rootContainerElement: any).nodeType !== DOCUMENT_NODE + ) { target = (rootContainerElement: any).ownerDocument; } if (enablePassiveEventIntervention && isPassiveListener === undefined) { @@ -426,48 +467,50 @@ export function listenToReactEvent( rootContainerElement: Element, targetElement: Element | null, ): void { - const dependencies = registrationNameDependencies[reactEvent]; - const dependenciesLength = dependencies.length; - // If the dependencies length is 1, that means we're not using a polyfill - // plugin like ChangeEventPlugin, BeforeInputPlugin, EnterLeavePlugin - // and SelectEventPlugin. We always use the native bubble event phase for - // these plugins and emulate two phase event dispatching. SimpleEventPlugin - // always only has a single dependency and SimpleEventPlugin events also - // use either the native capture event phase or bubble event phase, there - // is no emulation (except for focus/blur, but that will be removed soon). - const isPolyfillEventPlugin = dependenciesLength !== 1; + if (!enableEagerRootListeners) { + const dependencies = registrationNameDependencies[reactEvent]; + const dependenciesLength = dependencies.length; + // If the dependencies length is 1, that means we're not using a polyfill + // plugin like ChangeEventPlugin, BeforeInputPlugin, EnterLeavePlugin + // and SelectEventPlugin. We always use the native bubble event phase for + // these plugins and emulate two phase event dispatching. SimpleEventPlugin + // always only has a single dependency and SimpleEventPlugin events also + // use either the native capture event phase or bubble event phase, there + // is no emulation (except for focus/blur, but that will be removed soon). + const isPolyfillEventPlugin = dependenciesLength !== 1; - if (isPolyfillEventPlugin) { - const listenerMap = getEventListenerMap(rootContainerElement); - // For optimization, we register plugins on the listener map, so we - // don't need to check each of their dependencies each time. - if (!listenerMap.has(reactEvent)) { - listenerMap.set(reactEvent, null); - for (let i = 0; i < dependenciesLength; i++) { - listenToNativeEvent( - dependencies[i], - false, - rootContainerElement, - targetElement, - ); + if (isPolyfillEventPlugin) { + const listenerMap = getEventListenerMap(rootContainerElement); + // For optimization, we register plugins on the listener map, so we + // don't need to check each of their dependencies each time. + if (!listenerMap.has(reactEvent)) { + listenerMap.set(reactEvent, null); + for (let i = 0; i < dependenciesLength; i++) { + listenToNativeEvent( + dependencies[i], + false, + rootContainerElement, + targetElement, + ); + } } + } else { + const isCapturePhaseListener = + reactEvent.substr(-7) === 'Capture' && + // Edge case: onGotPointerCapture and onLostPointerCapture + // end with "Capture" but that's part of their event names. + // The Capture versions would end with CaptureCapture. + // So we have to check against that. + // This check works because none of the events we support + // end with "Pointer". + reactEvent.substr(-14, 7) !== 'Pointer'; + listenToNativeEvent( + dependencies[0], + isCapturePhaseListener, + rootContainerElement, + targetElement, + ); } - } else { - const isCapturePhaseListener = - reactEvent.substr(-7) === 'Capture' && - // Edge case: onGotPointerCapture and onLostPointerCapture - // end with "Capture" but that's part of their event names. - // The Capture versions would end with CaptureCapture. - // So we have to check against that. - // This check works because none of the events we support - // end with "Pointer". - reactEvent.substr(-14, 7) !== 'Pointer'; - listenToNativeEvent( - dependencies[0], - isCapturePhaseListener, - rootContainerElement, - targetElement, - ); } } diff --git a/packages/react-dom/src/events/EventRegistry.js b/packages/react-dom/src/events/EventRegistry.js index 3dd665623f..dfe938d113 100644 --- a/packages/react-dom/src/events/EventRegistry.js +++ b/packages/react-dom/src/events/EventRegistry.js @@ -9,6 +9,10 @@ import type {DOMEventName} from './DOMEventNames'; +import {enableEagerRootListeners} from 'shared/ReactFeatureFlags'; + +export const allNativeEvents: Set = new Set(); + /** * Mapping from registration name to event name */ @@ -25,7 +29,7 @@ export const possibleRegistrationNames = __DEV__ ? {} : (null: any); export function registerTwoPhaseEvent( registrationName: string, - dependencies: ?Array, + dependencies: Array, ): void { registerDirectEvent(registrationName, dependencies); registerDirectEvent(registrationName + 'Capture', dependencies); @@ -33,7 +37,7 @@ export function registerTwoPhaseEvent( export function registerDirectEvent( registrationName: string, - dependencies: ?Array, + dependencies: Array, ) { if (__DEV__) { if (registrationNameDependencies[registrationName]) { @@ -55,4 +59,10 @@ export function registerDirectEvent( possibleRegistrationNames.ondblclick = registrationName; } } + + if (enableEagerRootListeners) { + for (let i = 0; i < dependencies.length; i++) { + allNativeEvents.add(dependencies[i]); + } + } } diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index d92c6d5eb5..d8ff5300aa 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -32,6 +32,7 @@ import { import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags'; import { type EventSystemFlags, + IS_CAPTURE_PHASE, IS_LEGACY_FB_SUPPORT_MODE, } from './EventSystemFlags'; @@ -40,6 +41,7 @@ import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; import { enableLegacyFBSupport, + enableEagerRootListeners, decoupleUpdatePriorityFromScheduler, } from 'shared/ReactFeatureFlags'; import { @@ -191,7 +193,21 @@ export function dispatchEvent( if (!_enabled) { return; } - if (hasQueuedDiscreteEvents() && isReplayableDiscreteEvent(domEventName)) { + let allowReplay = true; + if (enableEagerRootListeners) { + // TODO: replaying capture phase events is currently broken + // because we used to do it during top-level native bubble handlers + // but now we use different bubble and capture handlers. + // In eager mode, we attach capture listeners early, so we need + // to filter them out until we fix the logic to handle them correctly. + // This could've been outside the flag but I put it inside to reduce risk. + allowReplay = (eventSystemFlags & IS_CAPTURE_PHASE) === 0; + } + if ( + allowReplay && + hasQueuedDiscreteEvents() && + isReplayableDiscreteEvent(domEventName) + ) { // If we already have a queue of discrete events, and this is another discrete // event, then we can't dispatch it regardless of its target, since they // need to dispatch in order. @@ -214,38 +230,40 @@ export function dispatchEvent( if (blockedOn === null) { // We successfully dispatched this event. + if (allowReplay) { + clearIfContinuousEvent(domEventName, nativeEvent); + } + return; + } + + if (allowReplay) { + if (isReplayableDiscreteEvent(domEventName)) { + // This this to be replayed later once the target is available. + queueDiscreteEvent( + blockedOn, + domEventName, + eventSystemFlags, + targetContainer, + nativeEvent, + ); + return; + } + if ( + queueIfContinuousEvent( + blockedOn, + domEventName, + eventSystemFlags, + targetContainer, + nativeEvent, + ) + ) { + return; + } + // We need to clear only if we didn't queue because + // queueing is accummulative. clearIfContinuousEvent(domEventName, nativeEvent); - return; } - if (isReplayableDiscreteEvent(domEventName)) { - // This this to be replayed later once the target is available. - queueDiscreteEvent( - blockedOn, - domEventName, - eventSystemFlags, - targetContainer, - nativeEvent, - ); - return; - } - - if ( - queueIfContinuousEvent( - blockedOn, - domEventName, - eventSystemFlags, - targetContainer, - nativeEvent, - ) - ) { - return; - } - - // We need to clear only if we didn't queue because - // queueing is accummulative. - clearIfContinuousEvent(domEventName, nativeEvent); - // This is not replayable so we'll invoke it but without a target, // in case the event system needs to trace it. dispatchEventForPluginEventSystem( diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index ded99d1961..8b8d08f5e0 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -14,7 +14,10 @@ import type {EventSystemFlags} from './EventSystemFlags'; import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes'; import type {LanePriority} from 'react-reconciler/src/ReactFiberLane'; -import {enableSelectiveHydration} from 'shared/ReactFeatureFlags'; +import { + enableSelectiveHydration, + enableEagerRootListeners, +} from 'shared/ReactFeatureFlags'; import { unstable_runWithPriority as runWithPriority, unstable_scheduleCallback as scheduleCallback, @@ -177,21 +180,27 @@ function trapReplayableEventForContainer( domEventName: DOMEventName, container: Container, ) { - listenToNativeEvent(domEventName, false, ((container: any): Element), null); + // When the flag is on, we do this in a unified codepath elsewhere. + if (!enableEagerRootListeners) { + listenToNativeEvent(domEventName, false, ((container: any): Element), null); + } } export function eagerlyTrapReplayableEvents( container: Container, document: Document, ) { - // Discrete - discreteReplayableEvents.forEach(domEventName => { - trapReplayableEventForContainer(domEventName, container); - }); - // Continuous - continuousReplayableEvents.forEach(domEventName => { - trapReplayableEventForContainer(domEventName, container); - }); + // When the flag is on, we do this in a unified codepath elsewhere. + if (!enableEagerRootListeners) { + // Discrete + discreteReplayableEvents.forEach(domEventName => { + trapReplayableEventForContainer(domEventName, container); + }); + // Continuous + continuousReplayableEvents.forEach(domEventName => { + trapReplayableEventForContainer(domEventName, container); + }); + } } function createQueuedReplayableEvent( diff --git a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js index abf1c14dc2..9638722378 100644 --- a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js @@ -2401,85 +2401,97 @@ describe('DOMPluginEventSystem', () => { ); let setCustomEventHandle; + if (gate(flags => flags.enableEagerRootListeners)) { + // With eager listeners, supporting custom events via this API doesn't make sense + // because we can't know a full list of them ahead of time. Let's check we throw + // since otherwise we'd end up with inconsistent behavior, like no portal bubbling. + expect(() => { + setCustomEventHandle = ReactDOM.unstable_createEventHandle( + 'custom-event', + ); + }).toThrow( + 'Cannot call unstable_createEventHandle with "custom-event", as it is not an event known to React.', + ); + } else { + // Test that we get a warning when we don't provide an explicit priority + expect(() => { + setCustomEventHandle = ReactDOM.unstable_createEventHandle( + 'custom-event', + ); + }).toWarnDev( + 'Warning: The event "custom-event" provided to createEventHandle() does not have a known priority type. ' + + 'It is recommended to provide a "priority" option to specify a priority.', + {withoutStack: true}, + ); - // Test that we get a warning when we don't provide an explicit priority - expect(() => { setCustomEventHandle = ReactDOM.unstable_createEventHandle( 'custom-event', + { + priority: 0, // Discrete + }, ); - }).toWarnDev( - 'Warning: The event "type" provided to createEventHandle() does not have a known priority type. ' + - 'It is recommended to provide a "priority" option to specify a priority.', - {withoutStack: true}, - ); - setCustomEventHandle = ReactDOM.unstable_createEventHandle( - 'custom-event', - { - priority: 0, // Discrete - }, - ); - - const setCustomCaptureHandle = ReactDOM.unstable_createEventHandle( - 'custom-event', - { - capture: true, - priority: 0, // Discrete - }, - ); - - function Test() { - React.useEffect(() => { - const clearCustom1 = setCustomEventHandle( - buttonRef.current, - onCustomEvent, - ); - const clearCustom2 = setCustomCaptureHandle( - buttonRef.current, - onCustomEventCapture, - ); - const clearCustom3 = setCustomEventHandle( - divRef.current, - onCustomEvent, - ); - const clearCustom4 = setCustomCaptureHandle( - divRef.current, - onCustomEventCapture, - ); - - return () => { - clearCustom1(); - clearCustom2(); - clearCustom3(); - clearCustom4(); - }; - }); - - return ( - + const setCustomCaptureHandle = ReactDOM.unstable_createEventHandle( + 'custom-event', + { + capture: true, + priority: 0, // Discrete + }, ); + + const Test = () => { + React.useEffect(() => { + const clearCustom1 = setCustomEventHandle( + buttonRef.current, + onCustomEvent, + ); + const clearCustom2 = setCustomCaptureHandle( + buttonRef.current, + onCustomEventCapture, + ); + const clearCustom3 = setCustomEventHandle( + divRef.current, + onCustomEvent, + ); + const clearCustom4 = setCustomCaptureHandle( + divRef.current, + onCustomEventCapture, + ); + + return () => { + clearCustom1(); + clearCustom2(); + clearCustom3(); + clearCustom4(); + }; + }); + + return ( + + ); + }; + + ReactDOM.render(, container); + Scheduler.unstable_flushAll(); + + const buttonElement = buttonRef.current; + dispatchEvent(buttonElement, 'custom-event'); + expect(onCustomEvent).toHaveBeenCalledTimes(1); + expect(onCustomEventCapture).toHaveBeenCalledTimes(1); + expect(log[0]).toEqual(['capture', buttonElement]); + expect(log[1]).toEqual(['bubble', buttonElement]); + + const divElement = divRef.current; + dispatchEvent(divElement, 'custom-event'); + expect(onCustomEvent).toHaveBeenCalledTimes(3); + expect(onCustomEventCapture).toHaveBeenCalledTimes(3); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); + expect(log[5]).toEqual(['bubble', buttonElement]); } - - ReactDOM.render(, container); - Scheduler.unstable_flushAll(); - - const buttonElement = buttonRef.current; - dispatchEvent(buttonElement, 'custom-event'); - expect(onCustomEvent).toHaveBeenCalledTimes(1); - expect(onCustomEventCapture).toHaveBeenCalledTimes(1); - expect(log[0]).toEqual(['capture', buttonElement]); - expect(log[1]).toEqual(['bubble', buttonElement]); - - const divElement = divRef.current; - dispatchEvent(divElement, 'custom-event'); - expect(onCustomEvent).toHaveBeenCalledTimes(3); - expect(onCustomEventCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', buttonElement]); - expect(log[3]).toEqual(['capture', divElement]); - expect(log[4]).toEqual(['bubble', divElement]); - expect(log[5]).toEqual(['bubble', buttonElement]); }); // @gate experimental @@ -2823,6 +2835,64 @@ describe('DOMPluginEventSystem', () => { expect(log[5]).toEqual(['bubble', buttonElement]); }); + // @gate experimental && enableEagerRootListeners + it('propagates known createEventHandle events through portals without inner listeners', () => { + const buttonRef = React.createRef(); + const divRef = React.createRef(); + const log = []; + const onClick = jest.fn(e => log.push(['bubble', e.currentTarget])); + const onClickCapture = jest.fn(e => + log.push(['capture', e.currentTarget]), + ); + const setClick = ReactDOM.unstable_createEventHandle('click'); + const setClickCapture = ReactDOM.unstable_createEventHandle( + 'click', + { + capture: true, + }, + ); + + const portalElement = document.createElement('div'); + document.body.appendChild(portalElement); + + function Child() { + return
Click me!
; + } + + function Parent() { + React.useEffect(() => { + const clear1 = setClick(buttonRef.current, onClick); + const clear2 = setClickCapture( + buttonRef.current, + onClickCapture, + ); + return () => { + clear1(); + clear2(); + }; + }); + + return ( + + ); + } + + ReactDOM.render(, container); + Scheduler.unstable_flushAll(); + + const divElement = divRef.current; + const buttonElement = buttonRef.current; + dispatchClickEvent(divElement); + expect(onClick).toHaveBeenCalledTimes(1); + expect(onClickCapture).toHaveBeenCalledTimes(1); + expect(log[0]).toEqual(['capture', buttonElement]); + expect(log[1]).toEqual(['bubble', buttonElement]); + + document.body.removeChild(portalElement); + }); + describe('Compatibility with Scopes API', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/react-dom/src/events/plugins/SelectEventPlugin.js b/packages/react-dom/src/events/plugins/SelectEventPlugin.js index bc5d7df1a4..7f8b72408f 100644 --- a/packages/react-dom/src/events/plugins/SelectEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SelectEventPlugin.js @@ -16,6 +16,7 @@ import {canUseDOM} from 'shared/ExecutionEnvironment'; import {SyntheticEvent} from '../../events/SyntheticEvent'; import isTextInputElement from '../isTextInputElement'; import shallowEqual from 'shared/shallowEqual'; +import {enableEagerRootListeners} from 'shared/ReactFeatureFlags'; import {registerTwoPhaseEvent} from '../EventRegistry'; import getActiveElement from '../../client/getActiveElement'; @@ -152,19 +153,21 @@ function extractEvents( eventSystemFlags: EventSystemFlags, targetContainer: EventTarget, ) { - const eventListenerMap = getEventListenerMap(targetContainer); - // Track whether all listeners exists for this plugin. If none exist, we do - // not extract events. See #3639. - if ( - // If we are handling selectionchange, then we don't need to - // check for the other dependencies, as selectionchange is only - // event attached from the onChange plugin and we don't expose an - // onSelectionChange event from React. - domEventName !== 'selectionchange' && - !eventListenerMap.has('onSelect') && - !eventListenerMap.has('onSelectCapture') - ) { - return; + if (!enableEagerRootListeners) { + const eventListenerMap = getEventListenerMap(targetContainer); + // Track whether all listeners exists for this plugin. If none exist, we do + // not extract events. See #3639. + if ( + // If we are handling selectionchange, then we don't need to + // check for the other dependencies, as selectionchange is only + // event attached from the onChange plugin and we don't expose an + // onSelectionChange event from React. + domEventName !== 'selectionchange' && + !eventListenerMap.has('onSelect') && + !eventListenerMap.has('onSelectCapture') + ) { + return; + } } const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; diff --git a/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js index d8eb2c9219..4614052f50 100644 --- a/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js +++ b/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js @@ -538,7 +538,18 @@ describe('SimpleEventPlugin', function() { ); if (gate(flags => flags.enablePassiveEventIntervention)) { - expect(passiveEvents).toEqual(['touchstart', 'touchmove', 'wheel']); + if (gate(flags => flags.enableEagerRootListeners)) { + expect(passiveEvents).toEqual([ + 'touchstart', + 'touchstart', + 'touchmove', + 'touchmove', + 'wheel', + 'wheel', + ]); + } else { + expect(passiveEvents).toEqual(['touchstart', 'touchmove', 'wheel']); + } } else { expect(passiveEvents).toEqual([]); } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 2129245913..1ae1c94e61 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -136,3 +136,5 @@ export const enableDiscreteEventFlushingChange = false; // https://github.com/facebook/react/pull/19654 export const enablePassiveEventIntervention = true; export const disableOnScrollBubbling = true; + +export const enableEagerRootListeners = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 078d1e6b0f..ed2b0952cb 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -51,6 +51,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 89512478b4..0137e89087 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 280fc35872..9481dfda8c 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index baee0a991e..4a710543cb 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -49,6 +49,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index aeb589d1b8..5efc2138d0 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 1ba4439b28..8a715c02f9 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index b1b37b557f..3dbbb37824 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = true; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index f58f9f8aae..3cf35c8607 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -21,6 +21,7 @@ export const decoupleUpdatePriorityFromScheduler = __VARIANT__; export const skipUnmountedBoundaries = __VARIANT__; export const enablePassiveEventIntervention = __VARIANT__; export const disableOnScrollBubbling = __VARIANT__; +export const enableEagerRootListeners = __VARIANT__; // Enable this flag to help with concurrent mode debugging. // It logs information to the console about React scheduling, rendering, and commit phases. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index d9c237b048..dbbaf0c70d 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -29,6 +29,7 @@ export const { skipUnmountedBoundaries, enablePassiveEventIntervention, disableOnScrollBubbling, + enableEagerRootListeners, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 9498d6e5ed..06174f33af 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -360,5 +360,6 @@ "368": "ReactDOM.createEventHandle: setListener called on an invalid target. Provide a valid EventTarget or an element managed by React.", "369": "ReactDOM.createEventHandle: setter called on an invalid target. Provide a valid EventTarget or an element managed by React.", "370": "ReactDOM.createEventHandle: setter called with an invalid callback. The callback must be a function.", - "371": "Text string must be rendered within a component.\n\nText: %s" + "371": "Text string must be rendered within a component.\n\nText: %s", + "372": "Cannot call unstable_createEventHandle with \"%s\", as it is not an event known to React." }