From db1f2266282f1fc21ed7e8dbbd678fb44981d18f Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 10 Mar 2020 15:23:47 +0000 Subject: [PATCH] Flare: Fix listener upgrade bug Flare: fix bug with event upgrading Fix Fix --- .../react-dom/src/client/ReactDOMComponent.js | 4 +- .../src/events/DOMLegacyEventPluginSystem.js | 6 +-- .../src/events/DOMModernPluginEventSystem.js | 10 ++-- .../src/events/ReactDOMEventListener.js | 52 ++++++++++++------- ...edDOMEventResponderSystem-test.internal.js | 35 +++++++++++++ 5 files changed, 75 insertions(+), 32 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 5cc6981d82aad..d572d37a6bc83 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -59,7 +59,7 @@ import { import {getListenerMapForElement} from '../events/DOMEventListenerMap'; import { addResponderEventSystemEvent, - removeActiveResponderEventSystemEvent, + removeTrappedPassiveEventListener, } from '../events/ReactDOMEventListener.js'; import {mediaEventTypes} from '../events/DOMTopLevelEventTypes'; import { @@ -1360,7 +1360,7 @@ export function listenToEventResponderEventTypes( const passiveKey = targetEventType + '_passive'; const passiveListener = listenerMap.get(passiveKey); if (passiveListener != null) { - removeActiveResponderEventSystemEvent( + removeTrappedPassiveEventListener( document, targetEventType, passiveListener, diff --git a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js index d7881ac6903a8..8d0b6c4cf2449 100644 --- a/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js +++ b/packages/react-dom/src/events/DOMLegacyEventPluginSystem.js @@ -39,7 +39,7 @@ import { getRawEventName, mediaEventTypes, } from './DOMTopLevelEventTypes'; -import {trapEventForPluginEventSystem} from './ReactDOMEventListener'; +import {addTrappedEventListener} from './ReactDOMEventListener'; /** * Summary of `DOMEventPluginSystem` event handling: @@ -368,12 +368,12 @@ export function legacyTrapBubbledEvent( topLevelType: DOMTopLevelEventType, element: Document | Element, ): void { - trapEventForPluginEventSystem(element, topLevelType, false); + addTrappedEventListener(element, topLevelType, false); } export function legacyTrapCapturedEvent( topLevelType: DOMTopLevelEventType, element: Document | Element, ): void { - trapEventForPluginEventSystem(element, topLevelType, true); + addTrappedEventListener(element, topLevelType, true); } diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js index daea82b498147..ab2de7672c2e0 100644 --- a/packages/react-dom/src/events/DOMModernPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMModernPluginEventSystem.js @@ -21,7 +21,7 @@ import {plugins} from 'legacy-events/EventPluginRegistry'; import {HostRoot, HostPortal} from 'shared/ReactWorkTags'; -import {trapEventForPluginEventSystem} from './ReactDOMEventListener'; +import {addTrappedEventListener} from './ReactDOMEventListener'; import getEventTarget from './getEventTarget'; import {getListenerMapForElement} from './DOMEventListenerMap'; import { @@ -149,11 +149,7 @@ export function listenToTopLevelEvent( ): void { if (!listenerMap.has(topLevelType)) { const isCapturePhase = capturePhaseEvents.has(topLevelType); - trapEventForPluginEventSystem( - rootContainerElement, - topLevelType, - isCapturePhase, - ); + addTrappedEventListener(rootContainerElement, topLevelType, isCapturePhase); listenerMap.set(topLevelType, null); } } @@ -196,7 +192,7 @@ function willDeferLaterForFBLegacyPrimer(nativeEvent: any): boolean { if (node.tagName === 'A' && validFBLegacyPrimerRels.has(node.rel)) { const legacyFBSupport = true; const isCapture = nativeEvent.eventPhase === 1; - trapEventForPluginEventSystem( + addTrappedEventListener( document, ((type: any): DOMTopLevelEventType), isCapture, diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 040029c46d7f4..52b285b915396 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -105,6 +105,7 @@ export function addResponderEventSystemEvent( } else { eventFlags |= IS_ACTIVE; } + let fbListener; // Check if interactive and wrap in discreteUpdates const listener = dispatchEvent.bind( null, @@ -113,39 +114,27 @@ export function addResponderEventSystemEvent( document, ); if (passiveBrowserEventsSupported) { - addEventCaptureListenerWithPassiveFlag( + fbListener = addEventCaptureListenerWithPassiveFlag( document, topLevelType, listener, passive, ); } else { - addEventCaptureListener(document, topLevelType, listener); - } - return listener; -} - -export function removeActiveResponderEventSystemEvent( - document: Document, - topLevelType: string, - listener: any => void, -) { - if (passiveBrowserEventsSupported) { - document.removeEventListener(topLevelType, listener, { - capture: true, - passive: false, - }); - } else { - document.removeEventListener(topLevelType, listener, true); + fbListener = addEventCaptureListener(document, topLevelType, listener); } + // If we have an fbListener, then use that. + // We'll only have one if we use the forked + // EventListener-www module in FB builds. + return fbListener || listener; } -export function trapEventForPluginEventSystem( +export function addTrappedEventListener( container: Document | Element, topLevelType: DOMTopLevelEventType, capture: boolean, legacyFBSupport?: boolean, -): void { +): any => void { let listener; let listenerWrapper; switch (getEventPriorityForPluginSystem(topLevelType)) { @@ -203,6 +192,29 @@ export function trapEventForPluginEventSystem( } else { fbListener = addEventBubbleListener(container, rawEventName, listener); } + // If we have an fbListener, then use that. + // We'll only have one if we use the forked + // EventListener-www module in FB builds. + return fbListener || listener; +} + +export function removeTrappedPassiveEventListener( + document: Document, + topLevelType: string, + listener: any => void, +) { + if (listener.remove != null) { + listener.remove(); + } else { + if (passiveBrowserEventsSupported) { + document.removeEventListener(topLevelType, listener, { + capture: true, + passive: true, + }); + } else { + document.removeEventListener(topLevelType, listener, true); + } + } } function dispatchDiscreteEvent( diff --git a/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js index c3fa398e83766..62fda3edfb5c1 100644 --- a/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DeprecatedDOMEventResponderSystem-test.internal.js @@ -948,4 +948,39 @@ describe('DOMEventResponderSystem', () => { document.body.removeChild(domNode); expect(onEvent).toBeCalled(); }); + + it('event upgrading should work correctly', () => { + let eventResponderFiredCount = 0; + const buttonRef = React.createRef(); + + const TestResponder = createEventResponder({ + targetEventTypes: ['click'], + onEvent: (event, context, props, state) => { + eventResponderFiredCount++; + if (!state.addedRootEventTypes) { + context.addRootEventTypes(['click_active']); + } + state.addedRootEventTypes = true; + }, + }); + + function Test() { + const listener = React.DEPRECATED_useResponder(TestResponder, {}); + + return ( + + ); + } + + ReactDOM.render(, container); + expect(container.innerHTML).toBe(''); + + let buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + expect(eventResponderFiredCount).toBe(1); + dispatchClickEvent(buttonElement); + expect(eventResponderFiredCount).toBe(2); + }); });