From d5e4b3ae1d5f7d76bb3894e8d814fd4ba19dcfd5 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 2 Apr 2020 18:26:34 +0100 Subject: [PATCH] Modern Event System: refine flags and handling of enableLegacyFBSupport (#18466) --- packages/legacy-events/EventSystemFlags.js | 12 +++---- .../react-dom/src/__tests__/ReactDOM-test.js | 29 +++++++++++++--- .../src/__tests__/ReactDOMComponent-test.js | 33 ++++++++++++++----- .../src/client/ReactDOMHostConfig.js | 3 ++ .../src/events/DOMLegacyEventPluginSystem.js | 19 +++++++++-- .../src/events/DOMModernPluginEventSystem.js | 21 +++++++++--- .../src/events/ReactDOMEventListener.js | 20 +++++------ .../src/events/ReactDOMEventReplaying.js | 9 +++-- .../react-dom/src/events/SimpleEventPlugin.js | 5 +-- 9 files changed, 112 insertions(+), 39 deletions(-) diff --git a/packages/legacy-events/EventSystemFlags.js b/packages/legacy-events/EventSystemFlags.js index e31af6bae3..585519c117 100644 --- a/packages/legacy-events/EventSystemFlags.js +++ b/packages/legacy-events/EventSystemFlags.js @@ -11,10 +11,10 @@ export type EventSystemFlags = number; export const PLUGIN_EVENT_SYSTEM = 1; export const RESPONDER_EVENT_SYSTEM = 1 << 1; -export const IS_PASSIVE = 1 << 2; -export const IS_ACTIVE = 1 << 3; -export const PASSIVE_NOT_SUPPORTED = 1 << 4; -export const IS_REPLAYED = 1 << 5; -export const IS_FIRST_ANCESTOR = 1 << 6; -export const IS_TARGET_EVENT_ONLY = 1 << 7; +export const USE_EVENT_SYSTEM = 1 << 2; +export const IS_TARGET_PHASE_ONLY = 1 << 3; +export const IS_PASSIVE = 1 << 4; +export const PASSIVE_NOT_SUPPORTED = 1 << 5; +export const IS_REPLAYED = 1 << 6; +export const IS_FIRST_ANCESTOR = 1 << 7; export const LEGACY_FB_SUPPORT = 1 << 8; diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index c43ae76dc4..d8a6c6d2ff 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -13,6 +13,7 @@ let React; let ReactDOM; let ReactDOMServer; let ReactTestUtils; +const ReactFeatureFlags = require('shared/ReactFeatureFlags'); describe('ReactDOM', () => { beforeEach(() => { @@ -354,11 +355,31 @@ describe('ReactDOM', () => { document.body.appendChild(container); try { ReactDOM.render(, container); + let expected; + + if ( + ReactFeatureFlags.enableModernEventSystem & + ReactFeatureFlags.enableLegacyFBSupport + ) { + // We expect to duplicate the 2nd handler because this test is + // not really designed around how the legacy FB support system works. + // This is because the above test sync fires a click() event + // during that of another click event, which causes the FB support system + // to duplicate adding an event listener. In practice this would never + // happen, as we only apply the legacy FB logic for "click" events, + // which would never stack this way in product code. + expected = [ + '1st node clicked', + "2nd node clicked imperatively from 1st's handler", + "2nd node clicked imperatively from 1st's handler", + ]; + } else { + expected = [ + '1st node clicked', + "2nd node clicked imperatively from 1st's handler", + ]; + } - const expected = [ - '1st node clicked', - "2nd node clicked imperatively from 1st's handler", - ]; expect(actual).toEqual(expected); } finally { document.body.removeChild(container); diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 49dc7a3b3c..d7e856f1a7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -14,6 +14,7 @@ describe('ReactDOMComponent', () => { let ReactTestUtils; let ReactDOM; let ReactDOMServer; + const ReactFeatureFlags = require('shared/ReactFeatureFlags'); function normalizeCodeLocInfo(str) { return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); @@ -2595,14 +2596,30 @@ describe('ReactDOMComponent', () => { // might depend on this. // // @see https://github.com/facebook/react/pull/12919#issuecomment-395224674 - expect(eventOrder).toEqual([ - 'document capture', - 'inner capture', - 'inner bubble', - 'outer capture', - 'outer bubble', - 'document bubble', - ]); + if ( + ReactFeatureFlags.enableModernEventSystem & + ReactFeatureFlags.enableLegacyFBSupport + ) { + // The order will change here, as the legacy FB support adds + // the event listener onto the document after the one above has. + expect(eventOrder).toEqual([ + 'document capture', + 'document bubble', + 'inner capture', + 'inner bubble', + 'outer capture', + 'outer bubble', + ]); + } else { + expect(eventOrder).toEqual([ + 'document capture', + 'inner capture', + 'inner bubble', + 'outer capture', + 'outer bubble', + 'document bubble', + ]); + } } finally { document.body.removeChild(container); } diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 3927e3b61c..a8dae35dc8 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -76,6 +76,8 @@ import {HostComponent} from 'react-reconciler/src/ReactWorkTags'; import { RESPONDER_EVENT_SYSTEM, IS_PASSIVE, + PLUGIN_EVENT_SYSTEM, + USE_EVENT_SYSTEM, } from 'legacy-events/EventSystemFlags'; import { isManagedDOMElement, @@ -1156,6 +1158,7 @@ export function registerEvent( type, rootContainerInstance, listenerMap, + PLUGIN_EVENT_SYSTEM | USE_EVENT_SYSTEM, passive, priority, ); diff --git a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js index 412d2c94aa..7a76374160 100644 --- a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js +++ b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js @@ -21,7 +21,10 @@ import { HostComponent, HostText, } from 'react-reconciler/src/ReactWorkTags'; -import {IS_FIRST_ANCESTOR} from 'legacy-events/EventSystemFlags'; +import { + IS_FIRST_ANCESTOR, + PLUGIN_EVENT_SYSTEM, +} from 'legacy-events/EventSystemFlags'; import {batchedEventUpdates} from 'legacy-events/ReactGenericBatching'; import {runEventsInBatch} from 'legacy-events/EventBatching'; import {plugins} from 'legacy-events/EventPluginRegistry'; @@ -372,7 +375,12 @@ export function legacyTrapBubbledEvent( element: Document | Element, listenerMap?: ElementListenerMap, ): void { - const listener = addTrappedEventListener(element, topLevelType, false); + const listener = addTrappedEventListener( + element, + topLevelType, + PLUGIN_EVENT_SYSTEM, + false, + ); if (listenerMap) { listenerMap.set(topLevelType, {passive: undefined, listener}); } @@ -383,6 +391,11 @@ export function legacyTrapCapturedEvent( element: Document | Element, listenerMap: ElementListenerMap, ): void { - const listener = addTrappedEventListener(element, topLevelType, true); + const listener = addTrappedEventListener( + element, + topLevelType, + PLUGIN_EVENT_SYSTEM, + true, + ); listenerMap.set(topLevelType, {passive: undefined, listener}); } diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js index 0816640e53..f5419c5a13 100644 --- a/packages/react-dom/src/events/DOMModernPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMModernPluginEventSystem.js @@ -28,9 +28,11 @@ import {batchedEventUpdates} from 'legacy-events/ReactGenericBatching'; import {executeDispatchesInOrder} from 'legacy-events/EventPluginUtils'; import {plugins} from 'legacy-events/EventPluginRegistry'; import { + PLUGIN_EVENT_SYSTEM, LEGACY_FB_SUPPORT, IS_REPLAYED, - IS_TARGET_EVENT_ONLY, + IS_TARGET_PHASE_ONLY, + USE_EVENT_SYSTEM, } from 'legacy-events/EventSystemFlags'; import {HostRoot, HostPortal} from 'react-reconciler/src/ReactWorkTags'; @@ -229,6 +231,7 @@ export function listenToTopLevelEvent( topLevelType: DOMTopLevelEventType, targetContainer: EventTarget, listenerMap: ElementListenerMap, + eventSystemFlags: EventSystemFlags, passive?: boolean, priority?: EventPriority, capture?: boolean, @@ -265,6 +268,7 @@ export function listenToTopLevelEvent( const listener = addTrappedEventListener( targetContainer, topLevelType, + eventSystemFlags, isCapturePhase, false, passive, @@ -283,7 +287,12 @@ export function listenToEvent( for (let i = 0; i < dependencies.length; i++) { const dependency = dependencies[i]; - listenToTopLevelEvent(dependency, rootContainerElement, listenerMap); + listenToTopLevelEvent( + dependency, + rootContainerElement, + listenerMap, + PLUGIN_EVENT_SYSTEM, + ); } } @@ -301,6 +310,7 @@ function willDeferLaterForLegacyFBSupport( addTrappedEventListener( targetContainer, topLevelType, + PLUGIN_EVENT_SYSTEM | LEGACY_FB_SUPPORT, false, isDeferredListenerForLegacyFBSupport, ); @@ -343,10 +353,9 @@ export function dispatchEventForPluginEventSystem( ): void { let ancestorInst = targetInst; if (targetContainer !== null) { - if (eventTargetEventListenerStore.has(targetContainer)) { + if (eventSystemFlags & IS_TARGET_PHASE_ONLY) { // For TargetEvent nodes (i.e. document, window) ancestorInst = null; - eventSystemFlags |= IS_TARGET_EVENT_ONLY; } else { const targetContainerNode = ((targetContainer: any): Node); @@ -362,6 +371,8 @@ export function dispatchEventForPluginEventSystem( (eventSystemFlags & LEGACY_FB_SUPPORT) === 0 && // We also don't want to defer during event replaying. (eventSystemFlags & IS_REPLAYED) === 0 && + // We don't want to apply the legacy FB support for the useEvent API. + (eventSystemFlags & USE_EVENT_SYSTEM) === 0 && willDeferLaterForLegacyFBSupport(topLevelType, targetContainer) ) { return; @@ -483,6 +494,7 @@ export function attachListenerToManagedDOMElement( type, containerEventTarget, listenerMap, + PLUGIN_EVENT_SYSTEM | USE_EVENT_SYSTEM, passive, priority, ); @@ -522,6 +534,7 @@ export function attachTargetEventListener(listener: ReactDOMListener): void { type, eventTarget, listenerMap, + PLUGIN_EVENT_SYSTEM | USE_EVENT_SYSTEM | IS_TARGET_PHASE_ONLY, passive, priority, capture, diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 02336d66cb..4b1bfd3830 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -37,12 +37,11 @@ import { import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags'; import { type EventSystemFlags, + LEGACY_FB_SUPPORT, PLUGIN_EVENT_SYSTEM, RESPONDER_EVENT_SYSTEM, IS_PASSIVE, - IS_ACTIVE, PASSIVE_NOT_SUPPORTED, - LEGACY_FB_SUPPORT, } from 'legacy-events/EventSystemFlags'; import { @@ -103,12 +102,9 @@ export function addResponderEventSystemEvent( if (passiveBrowserEventsSupported) { eventFlags |= IS_PASSIVE; } else { - eventFlags |= IS_ACTIVE; eventFlags |= PASSIVE_NOT_SUPPORTED; passive = false; } - } else { - eventFlags |= IS_ACTIVE; } // Check if interactive and wrap in discreteUpdates const listener = dispatchEvent.bind( @@ -132,6 +128,7 @@ export function addResponderEventSystemEvent( export function addTrappedEventListener( targetContainer: EventTarget, topLevelType: DOMTopLevelEventType, + eventSystemFlags: EventSystemFlags, capture: boolean, isDeferredListenerForLegacyFBSupport?: boolean, passive?: boolean, @@ -160,10 +157,6 @@ export function addTrappedEventListener( if (passive === true && !passiveBrowserEventsSupported) { passive = false; } - const eventSystemFlags = - enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport - ? PLUGIN_EVENT_SYSTEM | LEGACY_FB_SUPPORT - : PLUGIN_EVENT_SYSTEM; listener = listenerWrapper.bind( null, @@ -268,7 +261,14 @@ function dispatchDiscreteEvent( container, nativeEvent, ) { - flushDiscreteUpdatesIfNeeded(nativeEvent.timeStamp); + if ( + !enableLegacyFBSupport || + // If we have Legacy FB support, it means we've already + // flushed for this event and we don't need to do it again. + (eventSystemFlags & LEGACY_FB_SUPPORT) === 0 + ) { + flushDiscreteUpdatesIfNeeded(nativeEvent.timeStamp); + } discreteUpdates( dispatchEvent, topLevelType, diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index 4dc3874a20..9e39586ecf 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -118,7 +118,7 @@ import { TOP_FOCUS, TOP_BLUR, } from './DOMTopLevelEventTypes'; -import {IS_REPLAYED} from 'legacy-events/EventSystemFlags'; +import {IS_REPLAYED, PLUGIN_EVENT_SYSTEM} from 'legacy-events/EventSystemFlags'; import {legacyListenToTopLevelEvent} from './DOMLegacyEventPluginSystem'; import {listenToTopLevelEvent} from './DOMModernPluginEventSystem'; @@ -219,7 +219,12 @@ function trapReplayableEventForContainer( container: Container, listenerMap: ElementListenerMap, ) { - listenToTopLevelEvent(topLevelType, ((container: any): Element), listenerMap); + listenToTopLevelEvent( + topLevelType, + ((container: any): Element), + listenerMap, + PLUGIN_EVENT_SYSTEM, + ); } function trapReplayableEventForDocument( diff --git a/packages/react-dom/src/events/SimpleEventPlugin.js b/packages/react-dom/src/events/SimpleEventPlugin.js index a2a0c9332f..f394c3dc75 100644 --- a/packages/react-dom/src/events/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/SimpleEventPlugin.js @@ -17,6 +17,7 @@ import type {PluginModule} from 'legacy-events/PluginModuleType'; import type {EventSystemFlags} from 'legacy-events/EventSystemFlags'; import SyntheticEvent from 'legacy-events/SyntheticEvent'; +import {IS_TARGET_PHASE_ONLY} from 'legacy-events/EventSystemFlags'; import * as DOMTopLevelEventTypes from './DOMTopLevelEventTypes'; import { @@ -38,7 +39,7 @@ import SyntheticWheelEvent from './SyntheticWheelEvent'; import getEventCharCode from './getEventCharCode'; import accumulateTwoPhaseListeners from './accumulateTwoPhaseListeners'; import accumulateEventTargetListeners from './accumulateEventTargetListeners'; -import {IS_TARGET_EVENT_ONLY} from 'legacy-events/EventSystemFlags'; + import {enableUseEventAPI} from 'shared/ReactFeatureFlags'; // Only used in DEV for exhaustiveness validation. @@ -210,7 +211,7 @@ const SimpleEventPlugin: PluginModule = { if ( enableUseEventAPI && eventSystemFlags !== undefined && - eventSystemFlags & IS_TARGET_EVENT_ONLY && + eventSystemFlags & IS_TARGET_PHASE_ONLY && targetContainer != null ) { accumulateEventTargetListeners(event, targetContainer);