From 6dbd8ceb4f61350f3a524dc93e4b712198e4bb7a 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 --- .../src/__tests__/ReactDOMComponent-test.js | 11 +- .../react-dom/src/client/ReactDOMComponent.js | 1 + .../src/client/ReactDOMEventHandle.js | 5 +- .../src/events/DOMModernPluginEventSystem.js | 356 ++++++++++++------ .../react-dom/src/events/PluginModuleType.js | 3 +- ...OMModernPluginEventSystem-test.internal.js | 50 +-- .../events/plugins/ModernChangeEventPlugin.js | 2 +- .../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, 338 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 64c76718e0a38..5c99adb0ac981 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, @@ -182,6 +181,7 @@ export function createEventHandle( topLevelType, passive, priority, + capture, ); } else if (enableScopeAPI && isReactScope(target)) { const scopeTarget = ((target: any): ReactScopeInstance); @@ -195,6 +195,7 @@ export function createEventHandle( 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 f1d95fe5a2922..867b7abbf6040 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([ @@ -214,6 +242,13 @@ export const capturePhaseEvents: Set = new Set([ TOP_WAITING, ]); +// Map these back to their originals +const alwaysBubblePhaseEvents = new Set([ + 'onChangeCapture', + 'onBeforeInputCapture', + 'onSelectCapture', +]); + if (enableCreateEventHandleAPI) { capturePhaseEvents.add(TOP_AFTER_BLUR); } @@ -231,36 +266,40 @@ function executeDispatch( function executeDispatchesInOrder( event: ReactSyntheticEvent, - capture: DispatchQueueItemPhase, - bubble: DispatchQueueItemPhase, + phase: DispatchQueueItemPhase, + eventSystemFlags: EventSystemFlags, ): void { + const inCapturePhase = eventSystemFlags & IS_CAPTURE_PHASE; 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 dispatchEventsInBatch( + dispatchQueue: DispatchQueue, + eventSystemFlags: EventSystemFlags, +): void { 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; + executeDispatchesInOrder(event, phase, eventSystemFlags); // Modern event system doesn't use pooling. } // This would be a good time to rethrow if any of the event handlers threw. @@ -285,7 +324,7 @@ function dispatchEventsForPlugins( eventSystemFlags, targetContainer, ); - dispatchEventsInBatch(dispatchQueue); + dispatchEventsInBatch(dispatchQueue, eventSystemFlags); } function shouldUpgradeListener( @@ -348,6 +387,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 +406,14 @@ export function listenToReactPropEvent( // this React prop event again. listenerMap.set(reactPropEvent, null); const dependencies = registrationNameDependencies[reactPropEvent]; + const registrationCapturePhase = + isCaptureRegistrationName(reactPropEvent) && + !alwaysBubblePhaseEvents.has(reactPropEvent); for (let i = 0; i < dependencies.length; i++) { const dependency = dependencies[i]; - const capture = capturePhaseEvents.has(dependency); - + const capture = + capturePhaseEvents.has(dependency) || registrationCapturePhase; listenToTopLevelEvent( dependency, rootContainerElement, @@ -381,7 +428,7 @@ function addTrappedEventListener( targetContainer: EventTarget, topLevelType: DOMTopLevelEventType, eventSystemFlags: EventSystemFlags, - capture: boolean, + capturePhase: boolean, isDeferredListenerForLegacyFBSupport?: boolean, passive?: boolean, priority?: EventPriority, @@ -427,12 +474,12 @@ function addTrappedEventListener( targetContainer, rawEventName, unsubscribeListener, - capture, + capturePhase, ); } }; } - if (capture) { + if (capturePhase) { if (enableCreateEventHandleAPI && passive !== undefined) { unsubscribeListener = addEventCaptureListenerWithPassiveFlag( targetContainer, @@ -524,6 +571,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 +671,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 +695,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 +709,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 +718,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 +758,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 +786,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 +812,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 +942,7 @@ function accumulateEnterLeaveListenersForEvent( if (registrationName === undefined) { return; } - const capturePhase: DispatchQueueItemPhase = []; - const bubblePhase: DispatchQueueItemPhase = []; + const phase: DispatchQueueItemPhase = []; let instance = target; while (instance !== null) { @@ -846,7 +958,7 @@ function accumulateEnterLeaveListenersForEvent( if (capture) { const captureListener = getListener(instance, registrationName); if (captureListener != null) { - capturePhase.push( + phase.unshift( createDispatchQueueItemPhaseEntry( instance, captureListener, @@ -854,10 +966,10 @@ function accumulateEnterLeaveListenersForEvent( ), ); } - } else { + } else if (!capture) { const bubbleListener = getListener(instance, registrationName); if (bubbleListener != null) { - bubblePhase.push( + phase.push( createDispatchQueueItemPhaseEntry( instance, bubbleListener, @@ -869,14 +981,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 +1026,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 +1038,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 9e5b09e8b0785..72bc918c2a244 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. @@ -2170,8 +2170,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 { @@ -2195,9 +2195,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..6dbca6050df89 100644 --- a/packages/react-dom/src/events/plugins/ModernChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ModernChangeEventPlugin.js @@ -106,7 +106,7 @@ function manualDispatchChangeEvent(nativeEvent) { } function runEventInBatch(dispatchQueue) { - dispatchEventsInBatch(dispatchQueue); + dispatchEventsInBatch(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;