From d8e5480a416739831d1a6ebe136ab691d3ce4e8d Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 30 Jun 2020 19:32:36 +0100 Subject: [PATCH] Modern Event System: make on*Capture events use capture phase WIP WIP FIX FIX Fix WIP Virtualize plugins Fix lint Clear up conflict Clearn things up Fix logic for plugins Fix test Simplify logic Simplify logic Optimize hot path Revise --- .../src/__tests__/ReactDOMComponent-test.js | 11 +- .../react-dom/src/client/ReactDOMComponent.js | 1 + .../src/client/ReactDOMEventHandle.js | 5 +- .../src/events/DOMModernPluginEventSystem.js | 352 ++++++++++++------ .../react-dom/src/events/PluginModuleType.js | 3 +- ...OMModernPluginEventSystem-test.internal.js | 50 +-- .../events/plugins/ModernChangeEventPlugin.js | 4 +- .../plugins/ModernEnterLeaveEventPlugin.js | 4 +- .../events/plugins/ModernSimpleEventPlugin.js | 12 +- .../__tests__/ModernChangeEventPlugin-test.js | 24 ++ .../__tests__/ModernSelectEventPlugin-test.js | 34 ++ .../src/legacy-events/PluginModuleType.js | 3 +- 12 files changed, 336 insertions(+), 167 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index db4ad0e847003..3c8ce165ebb90 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -2714,28 +2714,23 @@ describe('ReactDOMComponent', () => { innerRef.current.click(); - // The order we receive here is not ideal since it is expected that the - // capture listener fire before all bubble listeners. Other React apps - // might depend on this. - // - // @see https://github.com/facebook/react/pull/12919#issuecomment-395224674 if (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', + 'outer capture', 'inner capture', + 'document bubble', 'inner bubble', - 'outer capture', 'outer bubble', ]); } else { expect(eventOrder).toEqual([ 'document capture', + 'outer capture', 'inner capture', 'inner bubble', - 'outer capture', 'outer bubble', 'document bubble', ]); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 06ff7e52f8991..71a90cd3e61c0 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -13,6 +13,7 @@ import { registrationNameDependencies, possibleRegistrationNames, } from '../events/EventRegistry'; + import {canUseDOM} from 'shared/ExecutionEnvironment'; import invariant from 'shared/invariant'; import { diff --git a/packages/react-dom/src/client/ReactDOMEventHandle.js b/packages/react-dom/src/client/ReactDOMEventHandle.js index 1ce1a760cc575..3118d85c1e18a 100644 --- a/packages/react-dom/src/client/ReactDOMEventHandle.js +++ b/packages/react-dom/src/client/ReactDOMEventHandle.js @@ -26,7 +26,6 @@ import {ELEMENT_NODE} from '../shared/HTMLNodeType'; import { listenToTopLevelEvent, addEventTypeToDispatchConfig, - capturePhaseEvents, } from '../events/DOMModernPluginEventSystem'; import {HostRoot, HostPortal} from 'react-reconciler/src/ReactWorkTags'; @@ -87,6 +86,7 @@ function registerEventOnNearestTargetContainer( topLevelType: DOMTopLevelEventType, passive: boolean | void, priority: EventPriority | void, + capture: boolean, ): void { // If it is, find the nearest root or portal and make it // our event handle target container. @@ -99,7 +99,6 @@ function registerEventOnNearestTargetContainer( ); } const listenerMap = getEventListenerMap(targetContainer); - const capture = capturePhaseEvents.has(topLevelType); listenToTopLevelEvent( topLevelType, targetContainer, @@ -135,6 +134,7 @@ function registerReactDOMEvent( topLevelType, passive, priority, + capture, ); } else if (enableScopeAPI && isReactScope(target)) { const scopeTarget = ((target: any): ReactScopeInstance); @@ -148,6 +148,7 @@ function registerReactDOMEvent( topLevelType, passive, priority, + capture, ); } else if (isValidEventTarget(target)) { const eventTarget = ((target: any): EventTarget); diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js index ccfdf65469c31..f6100d8a38c17 100644 --- a/packages/react-dom/src/events/DOMModernPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMModernPluginEventSystem.js @@ -29,8 +29,8 @@ import { PLUGIN_EVENT_SYSTEM, LEGACY_FB_SUPPORT, IS_REPLAYED, - IS_TARGET_PHASE_ONLY, IS_CAPTURE_PHASE, + IS_TARGET_PHASE_ONLY, } from './EventSystemFlags'; import { @@ -131,6 +131,12 @@ function extractEvents( eventSystemFlags: EventSystemFlags, targetContainer: null | EventTarget, ) { + // TODO: we should remove the concept of a "SimpleEventPlugin". + // This is the basic functionality of the event system. All + // the other plugins are essentially polyfills. So the plugin + // should probably be inlined somewhere and have its logic + // be core the to event system. This would potentially allow + // us to ship builds of React without the polyfilled plugins below. ModernSimpleEventPlugin.extractEvents( dispatchQueue, topLevelType, @@ -140,42 +146,64 @@ function extractEvents( eventSystemFlags, targetContainer, ); - ModernEnterLeaveEventPlugin.extractEvents( - dispatchQueue, - topLevelType, - targetInst, - nativeEvent, - nativeEventTarget, - eventSystemFlags, - targetContainer, - ); - ModernChangeEventPlugin.extractEvents( - dispatchQueue, - topLevelType, - targetInst, - nativeEvent, - nativeEventTarget, - eventSystemFlags, - targetContainer, - ); - ModernSelectEventPlugin.extractEvents( - dispatchQueue, - topLevelType, - targetInst, - nativeEvent, - nativeEventTarget, - eventSystemFlags, - targetContainer, - ); - ModernBeforeInputEventPlugin.extractEvents( - dispatchQueue, - topLevelType, - targetInst, - nativeEvent, - nativeEventTarget, - eventSystemFlags, - targetContainer, - ); + const shouldProcessPolyfillPlugins = + (eventSystemFlags & IS_CAPTURE_PHASE) === 0 || + capturePhaseEvents.has(topLevelType); + // We don't process these events unless we are in the + // event's native "bubble" phase, which means that we're + // not in the capture phase. That's because we emulate + // the capture phase here still. This is a trade-off, + // because in an ideal world we would not emulate and use + // the phases properly, like we do with the SimpleEvent + // plugin. However, the plugins below either expect + // emulation (EnterLeave) or use state localized to that + // plugin (BeforeInput, Change, Select). The state in + // these modules complicates things, as you'll essentially + // get the case where the capture phase event might change + // state, only for the following bubble event to come in + // later and not trigger anything as the state now + // invalidates the heuristics of the event plugin. We + // could alter all these plugins to work in such ways, but + // that might cause other unknown side-effects that we + // can't forsee right now. + if (shouldProcessPolyfillPlugins) { + ModernEnterLeaveEventPlugin.extractEvents( + dispatchQueue, + topLevelType, + targetInst, + nativeEvent, + nativeEventTarget, + eventSystemFlags, + targetContainer, + ); + ModernChangeEventPlugin.extractEvents( + dispatchQueue, + topLevelType, + targetInst, + nativeEvent, + nativeEventTarget, + eventSystemFlags, + targetContainer, + ); + ModernSelectEventPlugin.extractEvents( + dispatchQueue, + topLevelType, + targetInst, + nativeEvent, + nativeEventTarget, + eventSystemFlags, + targetContainer, + ); + ModernBeforeInputEventPlugin.extractEvents( + dispatchQueue, + topLevelType, + targetInst, + nativeEvent, + nativeEventTarget, + eventSystemFlags, + targetContainer, + ); + } } export const capturePhaseEvents: Set = new Set([ @@ -229,38 +257,42 @@ function executeDispatch( event.currentTarget = null; } -function executeDispatchesInOrder( +function processDispatchQueueItemsInOrder( event: ReactSyntheticEvent, - capture: DispatchQueueItemPhase, - bubble: DispatchQueueItemPhase, + phase: DispatchQueueItemPhase, + inCapturePhase: boolean, ): void { let previousInstance; - // Dispatch capture phase first. - for (let i = capture.length - 1; i >= 0; i--) { - const {instance, currentTarget, listener} = capture[i]; - if (instance !== previousInstance && event.isPropagationStopped()) { - return; + if (inCapturePhase) { + for (let i = phase.length - 1; i >= 0; i--) { + const {instance, currentTarget, listener} = phase[i]; + if (instance !== previousInstance && event.isPropagationStopped()) { + return; + } + executeDispatch(event, listener, currentTarget); + previousInstance = instance; } - executeDispatch(event, listener, currentTarget); - previousInstance = instance; - } - previousInstance = undefined; - // Dispatch bubble phase second. - for (let i = 0; i < bubble.length; i++) { - const {instance, currentTarget, listener} = bubble[i]; - if (instance !== previousInstance && event.isPropagationStopped()) { - return; + } else { + for (let i = 0; i < phase.length; i++) { + const {instance, currentTarget, listener} = phase[i]; + if (instance !== previousInstance && event.isPropagationStopped()) { + return; + } + executeDispatch(event, listener, currentTarget); + previousInstance = instance; } - executeDispatch(event, listener, currentTarget); - previousInstance = instance; } } -export function dispatchEventsInBatch(dispatchQueue: DispatchQueue): void { +export function processDispatchQueue( + dispatchQueue: DispatchQueue, + eventSystemFlags: EventSystemFlags, +): void { + const inCapturePhase = (eventSystemFlags & IS_CAPTURE_PHASE) !== 0; for (let i = 0; i < dispatchQueue.length; i++) { const dispatchQueueItem: DispatchQueueItem = dispatchQueue[i]; - const {event, capture, bubble} = dispatchQueueItem; - executeDispatchesInOrder(event, capture, bubble); + const {event, phase} = dispatchQueueItem; + processDispatchQueueItemsInOrder(event, phase, inCapturePhase); // Modern event system doesn't use pooling. } // This would be a good time to rethrow if any of the event handlers threw. @@ -285,7 +317,7 @@ function dispatchEventsForPlugins( eventSystemFlags, targetContainer, ); - dispatchEventsInBatch(dispatchQueue); + processDispatchQueue(dispatchQueue, eventSystemFlags); } function shouldUpgradeListener( @@ -348,6 +380,11 @@ export function listenToTopLevelEvent( } } +function isCaptureRegistrationName(registrationName: string): boolean { + const len = registrationName.length; + return registrationName.substr(len - 7) === 'Capture'; +} + export function listenToReactPropEvent( reactPropEvent: string, rootContainerElement: Element, @@ -362,11 +399,19 @@ export function listenToReactPropEvent( // this React prop event again. listenerMap.set(reactPropEvent, null); const dependencies = registrationNameDependencies[reactPropEvent]; + 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. SimpleEventPlugin always only has a single dependency. + // Given this, we know that we never need to apply capture phase event + // listeners to anything other than the SimpleEventPlugin. + const registrationCapturePhase = + isCaptureRegistrationName(reactPropEvent) && dependenciesLength === 1; - for (let i = 0; i < dependencies.length; i++) { + for (let i = 0; i < dependenciesLength; i++) { const dependency = dependencies[i]; - const capture = capturePhaseEvents.has(dependency); - + const capture = + capturePhaseEvents.has(dependency) || registrationCapturePhase; listenToTopLevelEvent( dependency, rootContainerElement, @@ -524,6 +569,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 apply this during capture phase. + (eventSystemFlags & IS_CAPTURE_PHASE) === 0 && willDeferLaterForLegacyFBSupport(topLevelType, targetContainer) ) { return; @@ -622,26 +669,23 @@ function createDispatchQueueItemPhaseEntry( function createDispatchQueueItem( event: ReactSyntheticEvent, - capture: DispatchQueueItemPhase, - bubble: DispatchQueueItemPhase, + phase: DispatchQueueItemPhase, ): DispatchQueueItem { return { event, - capture, - bubble, + phase, }; } -export function accumulateTwoPhaseListeners( +export function accumulateSinglePhaseListeners( targetFiber: Fiber | null, dispatchQueue: DispatchQueue, event: ReactSyntheticEvent, - accumulateEventHandleListeners?: boolean, + inCapturePhase: boolean, ): void { const bubbled = event._reactName; const captured = bubbled !== null ? bubbled + 'Capture' : null; - const capturePhase: DispatchQueueItemPhase = []; - const bubblePhase: DispatchQueueItemPhase = []; + const phase: DispatchQueueItemPhase = []; // If we are not handling EventTarget only phase, then we're doing the // usual two phase accumulation using the React fiber tree to pick up @@ -649,6 +693,11 @@ export function accumulateTwoPhaseListeners( let instance = targetFiber; let lastHostComponent = null; const targetType = event.type; + // shouldEmulateTwoPhase is temporary till we can polyfill focus/blur to + // focusin/focusout. + const shouldEmulateTwoPhase = capturePhaseEvents.has( + ((targetType: any): DOMTopLevelEventType), + ); // Accumulate all instances and listeners via the target -> root path. while (instance !== null) { @@ -658,7 +707,7 @@ export function accumulateTwoPhaseListeners( const currentTarget = stateNode; lastHostComponent = currentTarget; // For Event Handle listeners - if (enableCreateEventHandleAPI && accumulateEventHandleListeners) { + if (enableCreateEventHandleAPI) { const listeners = getEventHandlerListeners(currentTarget); if (listeners !== null) { @@ -667,32 +716,35 @@ export function accumulateTwoPhaseListeners( const listener = listenersArr[i]; const {callback, capture, type} = listener; if (type === targetType) { - if (capture === true) { - capturePhase.push( + if (capture && inCapturePhase) { + phase.push( createDispatchQueueItemPhaseEntry( instance, callback, currentTarget, ), ); - } else { - bubblePhase.push( - createDispatchQueueItemPhaseEntry( - instance, - callback, - currentTarget, - ), + } else if (!capture) { + const entry = createDispatchQueueItemPhaseEntry( + instance, + callback, + currentTarget, ); + if (shouldEmulateTwoPhase) { + phase.unshift(entry); + } else if (!inCapturePhase) { + phase.push(entry); + } } } } } } // Standard React on* listeners, i.e. onClick prop - if (captured !== null) { + if (captured !== null && inCapturePhase) { const captureListener = getListener(instance, captured); if (captureListener != null) { - capturePhase.push( + phase.push( createDispatchQueueItemPhaseEntry( instance, captureListener, @@ -704,19 +756,21 @@ export function accumulateTwoPhaseListeners( if (bubbled !== null) { const bubbleListener = getListener(instance, bubbled); if (bubbleListener != null) { - bubblePhase.push( - createDispatchQueueItemPhaseEntry( - instance, - bubbleListener, - currentTarget, - ), + const entry = createDispatchQueueItemPhaseEntry( + instance, + bubbleListener, + currentTarget, ); + if (shouldEmulateTwoPhase) { + phase.unshift(entry); + } else if (!inCapturePhase) { + phase.push(entry); + } } } } else if ( enableCreateEventHandleAPI && enableScopeAPI && - accumulateEventHandleListeners && tag === ScopeComponent && lastHostComponent !== null ) { @@ -730,22 +784,25 @@ export function accumulateTwoPhaseListeners( const listener = listenersArr[i]; const {callback, capture, type} = listener; if (type === targetType) { - if (capture === true) { - capturePhase.push( + if (capture && inCapturePhase) { + phase.push( createDispatchQueueItemPhaseEntry( instance, callback, lastCurrentTarget, ), ); - } else { - bubblePhase.push( - createDispatchQueueItemPhaseEntry( - instance, - callback, - lastCurrentTarget, - ), + } else if (!capture) { + const entry = createDispatchQueueItemPhaseEntry( + instance, + callback, + lastCurrentTarget, ); + if (shouldEmulateTwoPhase) { + phase.unshift(entry); + } else if (!inCapturePhase) { + phase.push(entry); + } } } } @@ -753,10 +810,64 @@ export function accumulateTwoPhaseListeners( } instance = instance.return; } - if (capturePhase.length !== 0 || bubblePhase.length !== 0) { - dispatchQueue.push( - createDispatchQueueItem(event, capturePhase, bubblePhase), - ); + if (phase.length !== 0) { + dispatchQueue.push(createDispatchQueueItem(event, phase)); + } +} + +// We should only use this function for: +// - ModernBeforeInputEventPlugin +// - ModernChangeEventPlugin +// - ModernSelectEventPlugin +// This is because we only process these plugins +// in the bubble phase, so we need to accumulate two +// phase event listeners (via emulation). +export function accumulateTwoPhaseListeners( + targetFiber: Fiber | null, + dispatchQueue: DispatchQueue, + event: ReactSyntheticEvent, +): void { + const bubbled = event._reactName; + const captured = bubbled !== null ? bubbled + 'Capture' : null; + const phase: DispatchQueueItemPhase = []; + let instance = targetFiber; + + // Accumulate all instances and listeners via the target -> root path. + while (instance !== null) { + const {stateNode, tag} = instance; + // Handle listeners that are on HostComponents (i.e.
) + if (tag === HostComponent && stateNode !== null) { + const currentTarget = stateNode; + // Standard React on* listeners, i.e. onClick prop + if (captured !== null) { + const captureListener = getListener(instance, captured); + if (captureListener != null) { + phase.unshift( + createDispatchQueueItemPhaseEntry( + instance, + captureListener, + currentTarget, + ), + ); + } + } + if (bubbled !== null) { + const bubbleListener = getListener(instance, bubbled); + if (bubbleListener != null) { + phase.push( + createDispatchQueueItemPhaseEntry( + instance, + bubbleListener, + currentTarget, + ), + ); + } + } + } + instance = instance.return; + } + if (phase.length !== 0) { + dispatchQueue.push(createDispatchQueueItem(event, phase)); } } @@ -829,8 +940,7 @@ function accumulateEnterLeaveListenersForEvent( if (registrationName === undefined) { return; } - const capturePhase: DispatchQueueItemPhase = []; - const bubblePhase: DispatchQueueItemPhase = []; + const phase: DispatchQueueItemPhase = []; let instance = target; while (instance !== null) { @@ -846,7 +956,7 @@ function accumulateEnterLeaveListenersForEvent( if (capture) { const captureListener = getListener(instance, registrationName); if (captureListener != null) { - capturePhase.push( + phase.unshift( createDispatchQueueItemPhaseEntry( instance, captureListener, @@ -854,10 +964,10 @@ function accumulateEnterLeaveListenersForEvent( ), ); } - } else { + } else if (!capture) { const bubbleListener = getListener(instance, registrationName); if (bubbleListener != null) { - bubblePhase.push( + phase.push( createDispatchQueueItemPhaseEntry( instance, bubbleListener, @@ -869,14 +979,17 @@ function accumulateEnterLeaveListenersForEvent( } instance = instance.return; } - if (capturePhase.length !== 0 || bubblePhase.length !== 0) { - dispatchQueue.push( - createDispatchQueueItem(event, capturePhase, bubblePhase), - ); + if (phase.length !== 0) { + dispatchQueue.push(createDispatchQueueItem(event, phase)); } } -export function accumulateEnterLeaveListeners( +// We should only use this function for: +// - ModernEnterLeaveEventPlugin +// This is because we only process this plugin +// in the bubble phase, so we need to accumulate two +// phase event listeners. +export function accumulateEnterLeaveTwoPhaseListeners( dispatchQueue: DispatchQueue, leaveEvent: ReactSyntheticEvent, enterEvent: null | ReactSyntheticEvent, @@ -911,8 +1024,7 @@ export function accumulateEventHandleTargetListeners( currentTarget: EventTarget, inCapturePhase: boolean, ): void { - const capturePhase: DispatchQueueItemPhase = []; - const bubblePhase: DispatchQueueItemPhase = []; + const phase: DispatchQueueItemPhase = []; const eventListeners = getEventHandlerListeners(currentTarget); if (eventListeners !== null) { @@ -924,21 +1036,19 @@ export function accumulateEventHandleTargetListeners( const {callback, capture, type} = listener; if (type === targetType) { if (inCapturePhase && capture) { - capturePhase.push( + phase.push( createDispatchQueueItemPhaseEntry(null, callback, currentTarget), ); } else if (!inCapturePhase && !capture) { - bubblePhase.push( + phase.push( createDispatchQueueItemPhaseEntry(null, callback, currentTarget), ); } } } } - if (capturePhase.length !== 0 || bubblePhase.length !== 0) { - dispatchQueue.push( - createDispatchQueueItem(event, capturePhase, bubblePhase), - ); + if (phase.length !== 0) { + dispatchQueue.push(createDispatchQueueItem(event, phase)); } } diff --git a/packages/react-dom/src/events/PluginModuleType.js b/packages/react-dom/src/events/PluginModuleType.js index a9102a48c9812..f8f20995b68a9 100644 --- a/packages/react-dom/src/events/PluginModuleType.js +++ b/packages/react-dom/src/events/PluginModuleType.js @@ -26,8 +26,7 @@ export type DispatchQueueItemPhase = Array; export type DispatchQueueItem = {| event: ReactSyntheticEvent, - capture: DispatchQueueItemPhase, - bubble: DispatchQueueItemPhase, + phase: DispatchQueueItemPhase, |}; export type DispatchQueue = Array; diff --git a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js index 195d6df3e554e..2ebdfa03de8e6 100644 --- a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js @@ -189,9 +189,9 @@ describe('DOMModernPluginEventSystem', () => { dispatchClickEvent(divElement); expect(onClick).toHaveBeenCalledTimes(3); expect(onClickCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', divElement]); - expect(log[3]).toEqual(['bubble', divElement]); - expect(log[4]).toEqual(['capture', buttonElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); expect(log[5]).toEqual(['bubble', buttonElement]); }); @@ -241,9 +241,9 @@ describe('DOMModernPluginEventSystem', () => { dispatchClickEvent(divElement); expect(onClick).toHaveBeenCalledTimes(3); expect(onClickCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', divElement]); - expect(log[3]).toEqual(['bubble', divElement]); - expect(log[4]).toEqual(['capture', buttonElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); expect(log[5]).toEqual(['bubble', buttonElement]); }); @@ -320,9 +320,9 @@ describe('DOMModernPluginEventSystem', () => { dispatchClickEvent(divElement); expect(onClick).toHaveBeenCalledTimes(3); expect(onClickCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', divElement]); - expect(log[3]).toEqual(['bubble', divElement]); - expect(log[4]).toEqual(['capture', buttonElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); expect(log[5]).toEqual(['bubble', buttonElement]); // Inside @@ -330,9 +330,9 @@ describe('DOMModernPluginEventSystem', () => { dispatchClickEvent(buttonElement2); expect(onClick).toHaveBeenCalledTimes(5); expect(onClickCapture).toHaveBeenCalledTimes(5); - expect(log[6]).toEqual(['capture', buttonElement2]); - expect(log[7]).toEqual(['bubble', buttonElement2]); - expect(log[8]).toEqual(['capture', buttonElement]); + expect(log[6]).toEqual(['capture', buttonElement]); + expect(log[7]).toEqual(['capture', buttonElement2]); + expect(log[8]).toEqual(['bubble', buttonElement2]); expect(log[9]).toEqual(['bubble', buttonElement]); }); @@ -385,9 +385,9 @@ describe('DOMModernPluginEventSystem', () => { dispatchClickEvent(divElement); expect(onClick).toHaveBeenCalledTimes(3); expect(onClickCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', divElement]); - expect(log[3]).toEqual(['bubble', divElement]); - expect(log[4]).toEqual(['capture', buttonElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); expect(log[5]).toEqual(['bubble', buttonElement]); }); @@ -442,9 +442,9 @@ describe('DOMModernPluginEventSystem', () => { dispatchClickEvent(divElement); expect(onClick).toHaveBeenCalledTimes(3); expect(onClickCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', divElement]); - expect(log[3]).toEqual(['bubble', divElement]); - expect(log[4]).toEqual(['capture', buttonElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); expect(log[5]).toEqual(['bubble', buttonElement]); }); @@ -757,7 +757,7 @@ describe('DOMModernPluginEventSystem', () => { const divElement = divRef.current; dispatchClickEvent(divElement); expect(onClick).toHaveBeenCalledTimes(1); - expect(onClickCapture).toHaveBeenCalledTimes(1); + expect(onClickCapture).toHaveBeenCalledTimes(3); document.body.removeChild(portalElement); }); @@ -1179,7 +1179,7 @@ describe('DOMModernPluginEventSystem', () => { if (enableLegacyFBSupport) { // We aren't using roots with legacyFBSupport, we put clicks on the document, so we exbit the previous // behavior. - expect(log).toEqual([]); + expect(log).toEqual(['capture root', 'capture portal']); } else { expect(log).toEqual([ // The events on root probably shouldn't fire if a non-React intermediated. but current behavior is that they do. @@ -2288,8 +2288,8 @@ describe('DOMModernPluginEventSystem', () => { if (enableLegacyFBSupport) { expect(log[0]).toEqual(['capture', window]); expect(log[1]).toEqual(['capture', document]); - expect(log[2]).toEqual(['bubble', document]); - expect(log[3]).toEqual(['capture', buttonElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['bubble', document]); expect(log[4]).toEqual(['bubble', buttonElement]); expect(log[5]).toEqual(['bubble', window]); } else { @@ -2313,9 +2313,9 @@ describe('DOMModernPluginEventSystem', () => { if (enableLegacyFBSupport) { expect(log[0]).toEqual(['capture', window]); expect(log[1]).toEqual(['capture', document]); - expect(log[2]).toEqual(['bubble', document]); - expect(log[3]).toEqual(['capture', buttonElement]); - expect(log[4]).toEqual(['capture', divElement]); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', document]); expect(log[5]).toEqual(['bubble', divElement]); expect(log[6]).toEqual(['bubble', buttonElement]); expect(log[7]).toEqual(['bubble', window]); diff --git a/packages/react-dom/src/events/plugins/ModernChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ModernChangeEventPlugin.js index 70459b175d825..ce9b87ba6b0b1 100644 --- a/packages/react-dom/src/events/plugins/ModernChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ModernChangeEventPlugin.js @@ -36,7 +36,7 @@ import {enqueueStateRestore} from '../ReactDOMControlledComponent'; import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags'; import {batchedUpdates} from '../ReactDOMUpdateBatching'; import { - dispatchEventsInBatch, + processDispatchQueue, accumulateTwoPhaseListeners, } from '../DOMModernPluginEventSystem'; @@ -106,7 +106,7 @@ function manualDispatchChangeEvent(nativeEvent) { } function runEventInBatch(dispatchQueue) { - dispatchEventsInBatch(dispatchQueue); + processDispatchQueue(dispatchQueue, 0); } function getInstIfValueChanged(targetInst: Object) { diff --git a/packages/react-dom/src/events/plugins/ModernEnterLeaveEventPlugin.js b/packages/react-dom/src/events/plugins/ModernEnterLeaveEventPlugin.js index a5cce2b7f9fc2..d06102b6d6a2f 100644 --- a/packages/react-dom/src/events/plugins/ModernEnterLeaveEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ModernEnterLeaveEventPlugin.js @@ -19,7 +19,7 @@ import { getClosestInstanceFromNode, getNodeFromInstance, } from '../../client/ReactDOMComponentTree'; -import {accumulateEnterLeaveListeners} from '../DOMModernPluginEventSystem'; +import {accumulateEnterLeaveTwoPhaseListeners} from '../DOMModernPluginEventSystem'; import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags'; import {getNearestMountedFiber} from 'react-reconciler/src/ReactFiberTreeReflection'; @@ -159,7 +159,7 @@ function extractEvents( enter = null; } - accumulateEnterLeaveListeners(dispatchQueue, leave, enter, from, to); + accumulateEnterLeaveTwoPhaseListeners(dispatchQueue, leave, enter, from, to); } export {registerEvents, extractEvents}; diff --git a/packages/react-dom/src/events/plugins/ModernSimpleEventPlugin.js b/packages/react-dom/src/events/plugins/ModernSimpleEventPlugin.js index be6731082c107..44daae1c50798 100644 --- a/packages/react-dom/src/events/plugins/ModernSimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ModernSimpleEventPlugin.js @@ -23,7 +23,7 @@ import { registerSimpleEvents, } from '../DOMEventProperties'; import { - accumulateTwoPhaseListeners, + accumulateSinglePhaseListeners, accumulateEventHandleTargetListeners, } from '../DOMModernPluginEventSystem'; import {IS_TARGET_PHASE_ONLY} from '../EventSystemFlags'; @@ -152,13 +152,13 @@ function extractEvents( nativeEventTarget, ); + const inCapturePhase = (eventSystemFlags & IS_CAPTURE_PHASE) !== 0; if ( enableCreateEventHandleAPI && eventSystemFlags !== undefined && eventSystemFlags & IS_TARGET_PHASE_ONLY && targetContainer != null ) { - const inCapturePhase = (eventSystemFlags & IS_CAPTURE_PHASE) !== 0; accumulateEventHandleTargetListeners( dispatchQueue, event, @@ -166,7 +166,13 @@ function extractEvents( inCapturePhase, ); } else { - accumulateTwoPhaseListeners(targetInst, dispatchQueue, event, true); + // We traverse only capture or bubble phase listeners + accumulateSinglePhaseListeners( + targetInst, + dispatchQueue, + event, + inCapturePhase, + ); } return event; } diff --git a/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.js index 3c3290bfe2b1f..9f80e7329c510 100644 --- a/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/plugins/__tests__/ModernChangeEventPlugin-test.js @@ -98,6 +98,30 @@ describe('ChangeEventPlugin', () => { } }); + it('should consider initial text value to be current (capture)', () => { + let called = 0; + + function cb(e) { + called++; + expect(e.type).toBe('change'); + } + + const node = ReactDOM.render( + , + container, + ); + node.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); + + if (ReactFeatureFlags.disableInputAttributeSyncing) { + // TODO: figure out why. This might be a bug. + expect(called).toBe(1); + } else { + // There should be no React change events because the value stayed the same. + expect(called).toBe(0); + } + }); + it('should consider initial checkbox checked=true to be current', () => { let called = 0; diff --git a/packages/react-dom/src/events/plugins/__tests__/ModernSelectEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/ModernSelectEventPlugin-test.js index fc1383d8f5977..538f3be346af2 100644 --- a/packages/react-dom/src/events/plugins/__tests__/ModernSelectEventPlugin-test.js +++ b/packages/react-dom/src/events/plugins/__tests__/ModernSelectEventPlugin-test.js @@ -108,6 +108,40 @@ describe('SelectEventPlugin', () => { expect(select).toHaveBeenCalledTimes(1); }); + it('should fire `onSelectCapture` when a listener is present', () => { + const select = jest.fn(); + const onSelectCapture = event => { + expect(typeof event).toBe('object'); + expect(event.type).toBe('select'); + expect(event.target).toBe(node); + select(event.currentTarget); + }; + + const node = ReactDOM.render( + , + container, + ); + node.focus(); + + let nativeEvent = new MouseEvent('focus', { + bubbles: true, + cancelable: true, + }); + node.dispatchEvent(nativeEvent); + expect(select).toHaveBeenCalledTimes(0); + + nativeEvent = new MouseEvent('mousedown', { + bubbles: true, + cancelable: true, + }); + node.dispatchEvent(nativeEvent); + expect(select).toHaveBeenCalledTimes(0); + + nativeEvent = new MouseEvent('mouseup', {bubbles: true, cancelable: true}); + node.dispatchEvent(nativeEvent); + expect(select).toHaveBeenCalledTimes(1); + }); + // Regression test for https://github.com/facebook/react/issues/11379 it('should not wait for `mouseup` after receiving `dragend`', () => { const select = jest.fn(); diff --git a/packages/react-native-renderer/src/legacy-events/PluginModuleType.js b/packages/react-native-renderer/src/legacy-events/PluginModuleType.js index 7f29ccc642f4f..b7d2e325200f9 100644 --- a/packages/react-native-renderer/src/legacy-events/PluginModuleType.js +++ b/packages/react-native-renderer/src/legacy-events/PluginModuleType.js @@ -45,8 +45,7 @@ export type DispatchQueueItemPhase = Array; export type DispatchQueueItem = {| event: ReactSyntheticEvent, - capture: DispatchQueueItemPhase, - bubble: DispatchQueueItemPhase, + phase: DispatchQueueItemPhase, |}; export type DispatchQueue = Array;