From 4e3545fd6fc62842d7dbf772923214474f59871a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 23 Apr 2020 20:19:28 +0100 Subject: [PATCH] ReactDOMEventListener: clean up module (#18713) --- .../react-dom/src/client/ReactDOMComponent.js | 10 +- .../src/events/DOMLegacyEventPluginSystem.js | 21 ++- .../src/events/DOMModernPluginEventSystem.js | 140 +++++++++++------- .../DeprecatedDOMEventResponderSystem.js | 57 +++++++ .../src/events/ReactDOMEventListener.js | 128 ++-------------- .../src/events/ReactDOMEventReplaying.js | 6 +- .../ModernChangeEventPlugin-test.internal.js | 32 ++-- .../ModernSimpleEventPlugin-test.internal.js | 3 +- 8 files changed, 193 insertions(+), 204 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index be97a0252d..5b355e1bd4 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -10,7 +10,11 @@ import {registrationNameModules} from 'legacy-events/EventPluginRegistry'; import {canUseDOM} from 'shared/ExecutionEnvironment'; import invariant from 'shared/invariant'; -import {setListenToResponderEventTypes} from '../events/DeprecatedDOMEventResponderSystem'; +import { + setListenToResponderEventTypes, + addResponderEventSystemEvent, + removeTrappedEventListener, +} from '../events/DeprecatedDOMEventResponderSystem'; import { getValueForAttribute, @@ -56,10 +60,6 @@ import { TOP_TOGGLE, } from '../events/DOMTopLevelEventTypes'; import {getListenerMapForElement} from '../events/DOMEventListenerMap'; -import { - addResponderEventSystemEvent, - removeTrappedEventListener, -} from '../events/ReactDOMEventListener.js'; import {mediaEventTypes} from '../events/DOMTopLevelEventTypes'; import { createDangerousStringForStyles, diff --git a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js index bf46b68474..2354e50dc0 100644 --- a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js +++ b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js @@ -44,9 +44,10 @@ import { getRawEventName, mediaEventTypes, } from './DOMTopLevelEventTypes'; -import {addTrappedEventListener} from './ReactDOMEventListener'; +import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener'; import {batchedEventUpdates} from './ReactDOMUpdateBatching'; import getListener from './getListener'; +import {addEventCaptureListener, addEventBubbleListener} from './EventListener'; /** * Summary of `DOMEventPluginSystem` event handling: @@ -399,6 +400,24 @@ export function legacyTrapCapturedEvent( listenerMap.set(topLevelType, {passive: undefined, listener}); } +function addTrappedEventListener( + targetContainer: EventTarget, + topLevelType: DOMTopLevelEventType, + eventSystemFlags: EventSystemFlags, + capture: boolean, +): any => void { + const rawEventName = getRawEventName(topLevelType); + const listener = createEventListenerWrapperWithPriority( + targetContainer, + topLevelType, + eventSystemFlags, + ); + const unsubscribeListener = capture + ? addEventCaptureListener(targetContainer, rawEventName, listener) + : addEventBubbleListener(targetContainer, rawEventName, listener); + return unsubscribeListener; +} + function getParent(inst: Object | null): Object | null { if (!inst) { return null; diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js index c347e718bb..85780aa3d2 100644 --- a/packages/react-dom/src/events/DOMModernPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMModernPluginEventSystem.js @@ -35,10 +35,6 @@ import { HostComponent, } from 'react-reconciler/src/ReactWorkTags'; -import { - addTrappedEventListener, - removeTrappedEventListener, -} from './ReactDOMEventListener'; import getEventTarget from './getEventTarget'; import {getListenerMapForElement} from './DOMEventListenerMap'; import { @@ -76,17 +72,25 @@ import { TOP_PLAYING, TOP_CLICK, TOP_SELECTION_CHANGE, + getRawEventName, } from './DOMTopLevelEventTypes'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; import {COMMENT_NODE} from '../shared/HTMLNodeType'; import {batchedEventUpdates} from './ReactDOMUpdateBatching'; import getListener from './getListener'; +import {passiveBrowserEventsSupported} from './checkPassiveEvents'; import {enableLegacyFBSupport} from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, rethrowCaughtError, } from 'shared/ReactErrorUtils'; +import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener'; +import { + removeEventListener, + addEventCaptureListener, + addEventBubbleListener, +} from './EventListener'; const capturePhaseEvents = new Set([ TOP_FOCUS, @@ -219,25 +223,6 @@ function dispatchEventsForPlugins( dispatchEventsInBatch(syntheticEvents); } -function shouldUpgradeListener( - listenerEntry: void | ElementListenerMapEntry, - passive: void | boolean, -): boolean { - if (listenerEntry === undefined) { - return false; - } - // Upgrade from passive to active. - if (passive !== true && listenerEntry.passive) { - return true; - } - // Upgrade from default-active (browser default) to active. - if (passive === false && listenerEntry.passive === undefined) { - return true; - } - // Otherwise, do not upgrade - return false; -} - export function listenToTopLevelEvent( topLevelType: DOMTopLevelEventType, targetContainer: EventTarget, @@ -247,21 +232,6 @@ export function listenToTopLevelEvent( priority?: EventPriority, capture?: boolean, ): void { - // If we explicitly define capture, then these are for EventTarget objects, - // rather than React managed DOM elements. So we need to ensure we separate - // capture and non-capture events. For React managed DOM nodes we only use - // one or the other, never both. Which one we use is determined by the the - // capturePhaseEvents Set (in this module) that defines if the event listener - // should use the capture phase – otherwise we always use the bubble phase. - // Finally, when we get to dispatching and accumulating event listeners, we - // check if the user wanted capture/bubble and emulate the behavior at that - // point (we call this accumulating two phase listeners). - const typeStr = ((topLevelType: any): string); - const listenerMapKey = - capture === undefined - ? topLevelType - : `${typeStr}_${capture ? 'capture' : 'bubble'}`; - // TOP_SELECTION_CHANGE needs to be attached to the document // otherwise it won't capture incoming events that are only // triggered on the document directly. @@ -269,21 +239,12 @@ export function listenToTopLevelEvent( targetContainer = (targetContainer: any).ownerDocument || targetContainer; listenerMap = getListenerMapForElement(targetContainer); } - const listenerEntry = listenerMap.get(listenerMapKey); - const shouldUpgrade = shouldUpgradeListener(listenerEntry, passive); - if (listenerEntry === undefined || shouldUpgrade) { - const isCapturePhase = - capture === undefined ? capturePhaseEvents.has(topLevelType) : capture; - // If we should upgrade, then we need to remove the existing trapped - // event listener for the target container. - if (shouldUpgrade) { - removeTrappedEventListener( - targetContainer, - topLevelType, - isCapturePhase, - ((listenerEntry: any): ElementListenerMapEntry).listener, - ); - } + const listenerEntry: ElementListenerMapEntry | void = listenerMap.get( + topLevelType, + ); + const isCapturePhase = + capture === undefined ? capturePhaseEvents.has(topLevelType) : capture; + if (listenerEntry === undefined) { const listener = addTrappedEventListener( targetContainer, topLevelType, @@ -293,7 +254,7 @@ export function listenToTopLevelEvent( passive, priority, ); - listenerMap.set(listenerMapKey, {passive, listener}); + listenerMap.set(topLevelType, {passive, listener}); } } @@ -315,6 +276,77 @@ export function listenToEvent( } } +function addTrappedEventListener( + targetContainer: EventTarget, + topLevelType: DOMTopLevelEventType, + eventSystemFlags: EventSystemFlags, + capture: boolean, + isDeferredListenerForLegacyFBSupport?: boolean, + passive?: boolean, + priority?: EventPriority, +): any => void { + let listener = createEventListenerWrapperWithPriority( + targetContainer, + topLevelType, + eventSystemFlags, + priority, + ); + // If passive option is not supported, then the event will be + // active and not passive. + if (passive === true && !passiveBrowserEventsSupported) { + passive = false; + } + + targetContainer = + enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport + ? (targetContainer: any).ownerDocument + : targetContainer; + + const rawEventName = getRawEventName(topLevelType); + + let unsubscribeListener; + // When legacyFBSupport is enabled, it's for when we + // want to add a one time event listener to a container. + // This should only be used with enableLegacyFBSupport + // due to requirement to provide compatibility with + // internal FB www event tooling. This works by removing + // the event listener as soon as it is invoked. We could + // also attempt to use the {once: true} param on + // addEventListener, but that requires support and some + // browsers do not support this today, and given this is + // to support legacy code patterns, it's likely they'll + // need support for such browsers. + if (enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport) { + const originalListener = listener; + listener = function(...p) { + try { + return originalListener.apply(this, p); + } finally { + removeEventListener( + targetContainer, + rawEventName, + unsubscribeListener, + capture, + ); + } + }; + } + if (capture) { + unsubscribeListener = addEventCaptureListener( + targetContainer, + rawEventName, + listener, + ); + } else { + unsubscribeListener = addEventBubbleListener( + targetContainer, + rawEventName, + listener, + ); + } + return unsubscribeListener; +} + function willDeferLaterForLegacyFBSupport( topLevelType: DOMTopLevelEventType, targetContainer: EventTarget, diff --git a/packages/react-dom/src/events/DeprecatedDOMEventResponderSystem.js b/packages/react-dom/src/events/DeprecatedDOMEventResponderSystem.js index 1f1f4dfb17..07175d3b65 100644 --- a/packages/react-dom/src/events/DeprecatedDOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DeprecatedDOMEventResponderSystem.js @@ -10,6 +10,7 @@ import { type EventSystemFlags, IS_PASSIVE, PASSIVE_NOT_SUPPORTED, + RESPONDER_EVENT_SYSTEM, } from './EventSystemFlags'; import type {AnyNativeEvent} from 'legacy-events/PluginModuleType'; import { @@ -37,6 +38,15 @@ import invariant from 'shared/invariant'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; import {enqueueStateRestore} from './ReactDOMControlledComponent'; +import {createEventListenerWrapper} from './ReactDOMEventListener'; +import {passiveBrowserEventsSupported} from './checkPassiveEvents'; +import {getRawEventName} from './DOMTopLevelEventTypes'; +import { + addEventCaptureListener, + addEventCaptureListenerWithPassiveFlag, + removeEventListener, +} from './EventListener'; + import { ContinuousEvent, UserBlockingEvent, @@ -571,3 +581,50 @@ function DEPRECATED_registerRootEventType( rootEventTypesSet.add(rootEventType); rootEventResponderInstances.add(eventResponderInstance); } + +export function addResponderEventSystemEvent( + document: Document, + topLevelType: string, + passive: boolean, +): any => void { + let eventFlags = RESPONDER_EVENT_SYSTEM; + + // If passive option is not supported, then the event will be + // active and not passive, but we flag it as using not being + // supported too. This way the responder event plugins know, + // and can provide polyfills if needed. + if (passive) { + if (passiveBrowserEventsSupported) { + eventFlags |= IS_PASSIVE; + } else { + eventFlags |= PASSIVE_NOT_SUPPORTED; + passive = false; + } + } + // Check if interactive and wrap in discreteUpdates + const listener = createEventListenerWrapper( + document, + ((topLevelType: any): DOMTopLevelEventType), + eventFlags, + ); + if (passiveBrowserEventsSupported) { + return addEventCaptureListenerWithPassiveFlag( + document, + topLevelType, + listener, + passive, + ); + } else { + return addEventCaptureListener(document, topLevelType, listener); + } +} + +export function removeTrappedEventListener( + targetContainer: EventTarget, + topLevelType: DOMTopLevelEventType, + capture: boolean, + listener: any => void, +): void { + const rawEventName = getRawEventName(topLevelType); + removeEventListener(targetContainer, rawEventName, listener, capture); +} diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 44a0a65b23..1eb5603513 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -36,20 +36,10 @@ import { LEGACY_FB_SUPPORT, PLUGIN_EVENT_SYSTEM, RESPONDER_EVENT_SYSTEM, - IS_PASSIVE, - PASSIVE_NOT_SUPPORTED, } from './EventSystemFlags'; -import { - addEventBubbleListener, - addEventCaptureListener, - addEventCaptureListenerWithPassiveFlag, - removeEventListener, -} from './EventListener'; import getEventTarget from './getEventTarget'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; -import {getRawEventName} from './DOMTopLevelEventTypes'; -import {passiveBrowserEventsSupported} from './checkPassiveEvents'; import { enableDeprecatedFlareAPI, @@ -85,58 +75,29 @@ export function isEnabled() { return _enabled; } -export function addResponderEventSystemEvent( - document: Document, - topLevelType: string, - passive: boolean, -): any => void { - let eventFlags = RESPONDER_EVENT_SYSTEM; - - // If passive option is not supported, then the event will be - // active and not passive, but we flag it as using not being - // supported too. This way the responder event plugins know, - // and can provide polyfills if needed. - if (passive) { - if (passiveBrowserEventsSupported) { - eventFlags |= IS_PASSIVE; - } else { - eventFlags |= PASSIVE_NOT_SUPPORTED; - passive = false; - } - } - // Check if interactive and wrap in discreteUpdates - const listener = dispatchEvent.bind( +export function createEventListenerWrapper( + targetContainer: EventTarget, + topLevelType: DOMTopLevelEventType, + eventSystemFlags: EventSystemFlags, +): Function { + return dispatchEvent.bind( null, - ((topLevelType: any): DOMTopLevelEventType), - eventFlags, - document, + topLevelType, + eventSystemFlags, + targetContainer, ); - if (passiveBrowserEventsSupported) { - return addEventCaptureListenerWithPassiveFlag( - document, - topLevelType, - listener, - passive, - ); - } else { - return addEventCaptureListener(document, topLevelType, listener); - } } -export function addTrappedEventListener( +export function createEventListenerWrapperWithPriority( targetContainer: EventTarget, topLevelType: DOMTopLevelEventType, eventSystemFlags: EventSystemFlags, - capture: boolean, - isDeferredListenerForLegacyFBSupport?: boolean, - passive?: boolean, priority?: EventPriority, -): any => void { +): Function { const eventPriority = priority === undefined ? getEventPriorityForPluginSystem(topLevelType) : priority; - let listener; let listenerWrapper; switch (eventPriority) { case DiscreteEvent: @@ -150,77 +111,12 @@ export function addTrappedEventListener( listenerWrapper = dispatchEvent; break; } - // If passive option is not supported, then the event will be - // active and not passive. - if (passive === true && !passiveBrowserEventsSupported) { - passive = false; - } - - listener = listenerWrapper.bind( + return listenerWrapper.bind( null, topLevelType, eventSystemFlags, targetContainer, ); - - targetContainer = - enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport - ? (targetContainer: any).ownerDocument - : targetContainer; - - const rawEventName = getRawEventName(topLevelType); - - let unsubscribeListener; - // When legacyFBSupport is enabled, it's for when we - // want to add a one time event listener to a container. - // This should only be used with enableLegacyFBSupport - // due to requirement to provide compatibility with - // internal FB www event tooling. This works by removing - // the event listener as soon as it is invoked. We could - // also attempt to use the {once: true} param on - // addEventListener, but that requires support and some - // browsers do not support this today, and given this is - // to support legacy code patterns, it's likely they'll - // need support for such browsers. - if (enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport) { - const originalListener = listener; - listener = function(...p) { - try { - return originalListener.apply(this, p); - } finally { - removeEventListener( - targetContainer, - rawEventName, - unsubscribeListener, - capture, - ); - } - }; - } - if (capture) { - unsubscribeListener = addEventCaptureListener( - targetContainer, - rawEventName, - listener, - ); - } else { - unsubscribeListener = addEventBubbleListener( - targetContainer, - rawEventName, - listener, - ); - } - return unsubscribeListener; -} - -export function removeTrappedEventListener( - targetContainer: EventTarget, - topLevelType: DOMTopLevelEventType, - capture: boolean, - listener: any => void, -): void { - const rawEventName = getRawEventName(topLevelType); - removeEventListener(targetContainer, rawEventName, listener, capture); } function dispatchDiscreteEvent( diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index c29a9e88de..7954b6727c 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -30,10 +30,7 @@ import { getContainerFromFiber, getSuspenseInstanceFromFiber, } from 'react-reconciler/src/ReactFiberTreeReflection'; -import { - attemptToDispatchEvent, - addResponderEventSystemEvent, -} from './ReactDOMEventListener'; +import {attemptToDispatchEvent} from './ReactDOMEventListener'; import {getListenerMapForElement} from './DOMEventListenerMap'; import { getInstanceFromNode, @@ -121,6 +118,7 @@ import { import {IS_REPLAYED, PLUGIN_EVENT_SYSTEM} from './EventSystemFlags'; import {legacyListenToTopLevelEvent} from './DOMLegacyEventPluginSystem'; import {listenToTopLevelEvent} from './DOMModernPluginEventSystem'; +import {addResponderEventSystemEvent} from './DeprecatedDOMEventResponderSystem'; type QueuedReplayableEvent = {| blockedOn: null | Container | SuspenseInstance, diff --git a/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.internal.js b/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.internal.js index 7ada7be88e..b099d9e696 100644 --- a/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.internal.js @@ -9,9 +9,9 @@ 'use strict'; -let React = require('react'); -let ReactDOM = require('react-dom'); -let TestUtils = require('react-dom/test-utils'); +let React; +let ReactDOM; +let TestUtils; let ReactFeatureFlags; let Scheduler; @@ -34,6 +34,7 @@ describe('ChangeEventPlugin', () => { let container; beforeEach(() => { + jest.resetModules(); ReactFeatureFlags = require('shared/ReactFeatureFlags'); // TODO pull this into helper method, reduce repetition. // mock the browser APIs which are used in schedule: @@ -53,7 +54,11 @@ describe('ChangeEventPlugin', () => { postMessageCallback(postMessageEvent); } }; - jest.resetModules(); + ReactFeatureFlags.enableModernEventSystem = true; + React = require('react'); + ReactDOM = require('react-dom'); + TestUtils = require('react-dom/test-utils'); + Scheduler = require('scheduler'); container = document.createElement('div'); document.body.appendChild(container); }); @@ -467,17 +472,6 @@ describe('ChangeEventPlugin', () => { }); describe('concurrent mode', () => { - beforeEach(() => { - jest.resetModules(); - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.enableModernEventSystem = true; - - React = require('react'); - ReactDOM = require('react-dom'); - TestUtils = require('react-dom/test-utils'); - Scheduler = require('scheduler'); - }); - // @gate experimental it('text input', () => { const root = ReactDOM.createRoot(container); @@ -719,14 +713,6 @@ describe('ChangeEventPlugin', () => { // @gate experimental it('mouse enter/leave should be user-blocking but not discrete', async () => { - // This is currently behind a feature flag - jest.resetModules(); - ReactFeatureFlags.enableModernEventSystem = true; - React = require('react'); - ReactDOM = require('react-dom'); - TestUtils = require('react-dom/test-utils'); - Scheduler = require('scheduler'); - const {act} = TestUtils; const {useState} = React; diff --git a/packages/react-dom/src/events/plugins/__tests__/ModernSimpleEventPlugin-test.internal.js b/packages/react-dom/src/events/plugins/__tests__/ModernSimpleEventPlugin-test.internal.js index 598101683d..a8c02c96ce 100644 --- a/packages/react-dom/src/events/plugins/__tests__/ModernSimpleEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/plugins/__tests__/ModernSimpleEventPlugin-test.internal.js @@ -234,7 +234,8 @@ describe('SimpleEventPlugin', function() { describe('interactive events, in concurrent mode', () => { beforeEach(() => { jest.resetModules(); - + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableModernEventSystem = true; ReactDOM = require('react-dom'); Scheduler = require('scheduler'); });