From 2253bc81d052110e14fd0eeb335d47d580ace82b Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 8 Jul 2019 17:03:15 +0100 Subject: [PATCH] [Flare] Switch from currentTarget model to responderTarget model (#16082) --- .../src/events/DOMEventResponderSystem.js | 184 ++++++++---------- packages/react-events/src/dom/Focus.js | 2 +- packages/react-events/src/dom/Hover.js | 2 +- packages/react-events/src/dom/Press.js | 2 +- .../src/dom/__tests__/Press-test.internal.js | 28 +++ packages/shared/ReactDOMTypes.js | 2 +- 6 files changed, 116 insertions(+), 104 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 7097e9a27a84f..99b58a1d81550 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -503,12 +503,12 @@ function createDOMResponderEvent( } return { - currentTarget: nativeEventTarget, nativeEvent: nativeEvent, passive, passiveSupported, pointerId, pointerType: eventPointerType, + responderTarget: null, target: nativeEventTarget, type: topLevelType, }; @@ -580,81 +580,43 @@ function responderEventTypesContainType( return false; } -function handleTargetEventResponderInstance( +function validateEventTargetTypesForResponder( eventType: string, - responderEvent: ReactDOMResponderEvent, - eventComponentInstance: ReactDOMEventComponentInstance, - hookComponentResponderValidation: null | Set, - propagatedEventResponders: null | Set, -): void { - const responder = eventComponentInstance.responder; + responder: ReactDOMEventResponder, +): boolean { const targetEventTypes = responder.targetEventTypes; // Validate the target event type exists on the responder if (targetEventTypes !== undefined) { - if (responderEventTypesContainType(targetEventTypes, eventType)) { - if (hookComponentResponderValidation !== null) { - hookComponentResponderValidation.add(responder); - } - const {isHook, props, state} = eventComponentInstance; - const onEvent = responder.onEvent; - if (onEvent !== undefined) { - if ( - shouldSkipEventComponent( - eventComponentInstance, - ((responder: any): ReactDOMEventResponder), - propagatedEventResponders, - isHook, - ) - ) { - return; - } - currentInstance = eventComponentInstance; - currentlyInHook = isHook; - onEvent(responderEvent, eventResponderContext, props, state); - if (!isHook) { - checkForLocalPropagationContinuation( - responder, - ((propagatedEventResponders: any): Set), - ); - } - } - } - } -} - -function shouldSkipEventComponent( - eventResponderInstance: ReactDOMEventComponentInstance, - responder: ReactDOMEventResponder, - propagatedEventResponders: null | Set, - isHook: boolean, -): boolean { - if (propagatedEventResponders !== null && !isHook) { - if (propagatedEventResponders.has(responder)) { - return true; - } - propagatedEventResponders.add(responder); - } - if (globalOwner && globalOwner !== eventResponderInstance) { - return true; + return responderEventTypesContainType(targetEventTypes, eventType); } return false; } -function checkForLocalPropagationContinuation( +function handleTargetEventResponderInstance( + responderEvent: ReactDOMResponderEvent, + eventComponentInstance: ReactDOMEventComponentInstance, responder: ReactDOMEventResponder, - propagatedEventResponders: Set, ): void { - if (continueLocalPropagation === true) { - propagatedEventResponders.delete(responder); - continueLocalPropagation = false; + const {isHook, props, state} = eventComponentInstance; + const onEvent = responder.onEvent; + if (onEvent !== undefined) { + currentInstance = eventComponentInstance; + currentlyInHook = isHook; + onEvent(responderEvent, eventResponderContext, props, state); } } +function validateOwnership( + eventComponentInstance: ReactDOMEventComponentInstance, +): boolean { + return globalOwner === null || globalOwner === eventComponentInstance; +} + function traverseAndHandleEventResponderInstances( topLevelType: string, targetFiber: null | Fiber, nativeEvent: AnyNativeEvent, - nativeEventTarget: EventTarget, + nativeEventTarget: Document | Element, eventSystemFlags: EventSystemFlags, ): void { const isPassiveEvent = (eventSystemFlags & IS_PASSIVE) !== 0; @@ -669,60 +631,87 @@ function traverseAndHandleEventResponderInstances( const responderEvent = createDOMResponderEvent( topLevelType, nativeEvent, - ((nativeEventTarget: any): Element | Document), + nativeEventTarget, isPassiveEvent, isPassiveSupported, ); - const propagatedEventResponders: Set = new Set(); - - // We use this to know if we should check add hooks. If there are - // no event targets, then we don't add the hook forms. - const hookComponentResponderValidation = new Set(); + const responderTargets = new Map(); + const allowLocalPropagation = new Set(); // Bubbled event phases have the notion of local propagation. // This means that the propgation chain can be stopped part of the the way // through processing event component instances. let node = targetFiber; + let currentTarget = nativeEventTarget; while (node !== null) { const {dependencies, stateNode, tag} = node; if (tag === HostComponent) { - responderEvent.currentTarget = stateNode; + currentTarget = stateNode; } else if (tag === EventComponent) { const eventComponentInstance = stateNode; - // Switch to the current fiber tree - node = eventComponentInstance.currentFiber; - handleTargetEventResponderInstance( - eventType, - responderEvent, - eventComponentInstance, - hookComponentResponderValidation, - propagatedEventResponders, - ); + if (validateOwnership(eventComponentInstance)) { + const responder = eventComponentInstance.responder; + let responderTarget = responderTargets.get(responder); + let skipCurrentNode = false; + + if (responderTarget === undefined) { + if (validateEventTargetTypesForResponder(eventType, responder)) { + responderTarget = currentTarget; + responderTargets.set(responder, currentTarget); + } else { + skipCurrentNode = true; + } + } else if (allowLocalPropagation.has(responder)) { + // TODO: remove continueLocalPropagation + allowLocalPropagation.delete(responder); + } else { + skipCurrentNode = true; + } + if (!skipCurrentNode) { + responderEvent.responderTarget = ((responderTarget: any): + | Document + | Element); + // Switch to the current fiber tree + node = eventComponentInstance.currentFiber; + handleTargetEventResponderInstance( + responderEvent, + eventComponentInstance, + responder, + ); + // TODO: remove continueLocalPropagation + if (continueLocalPropagation) { + continueLocalPropagation = false; + allowLocalPropagation.add(responder); + } + } + } } else if (tag === FunctionComponent && dependencies !== null) { const events = dependencies.events; if (events !== null) { for (let i = 0; i < events.length; i++) { const eventComponentInstance = events[i]; - if ( - hookComponentResponderValidation.has( - eventComponentInstance.responder, - ) - ) { - handleTargetEventResponderInstance( - eventType, - responderEvent, - eventComponentInstance, - null, - null, - ); + if (validateOwnership(eventComponentInstance)) { + const responder = eventComponentInstance.responder; + const responderTarget = responderTargets.get(responder); + if (responderTarget !== undefined) { + responderEvent.responderTarget = responderTarget; + handleTargetEventResponderInstance( + responderEvent, + eventComponentInstance, + responder, + ); + // TODO: remove continueLocalPropagation + if (continueLocalPropagation) { + continueLocalPropagation = false; + allowLocalPropagation.add(responder); + } + } } } } } node = node.return; } - // Reset currentTarget to be null - responderEvent.currentTarget = null; // Root phase const rootEventInstances = rootEventTypesToEventComponentInstances.get( eventType, @@ -732,21 +721,16 @@ function traverseAndHandleEventResponderInstances( for (let i = 0; i < rootEventComponentInstances.length; i++) { const rootEventComponentInstance = rootEventComponentInstances[i]; + if (!validateOwnership(rootEventComponentInstance)) { + continue; + } const {isHook, props, responder, state} = rootEventComponentInstance; const onRootEvent = responder.onRootEvent; if (onRootEvent !== undefined) { - if ( - shouldSkipEventComponent( - rootEventComponentInstance, - responder, - null, - isHook, - ) - ) { - continue; - } currentInstance = rootEventComponentInstance; currentlyInHook = isHook; + const responderTarget = responderTargets.get(responder); + responderEvent.responderTarget = responderTarget || null; onRootEvent(responderEvent, eventResponderContext, props, state); } } @@ -858,7 +842,7 @@ export function dispatchEventForResponderEventSystem( topLevelType: string, targetFiber: null | Fiber, nativeEvent: AnyNativeEvent, - nativeEventTarget: EventTarget, + nativeEventTarget: Document | Element, eventSystemFlags: EventSystemFlags, ): void { if (enableFlareAPI) { diff --git a/packages/react-events/src/dom/Focus.js b/packages/react-events/src/dom/Focus.js index 04ee90fc7108c..f7b8661ff89c5 100644 --- a/packages/react-events/src/dom/Focus.js +++ b/packages/react-events/src/dom/Focus.js @@ -252,7 +252,7 @@ const FocusResponder: ReactDOMEventResponder = { if (!state.isFocused) { // Limit focus events to the direct child of the event component. // Browser focus is not expected to bubble. - state.focusTarget = event.currentTarget; + state.focusTarget = event.responderTarget; if (state.focusTarget === target) { state.isFocused = true; state.isLocalFocusVisible = isGlobalFocusVisible; diff --git a/packages/react-events/src/dom/Hover.js b/packages/react-events/src/dom/Hover.js index 6f95de69bfa2c..2afcdeb109c03 100644 --- a/packages/react-events/src/dom/Hover.js +++ b/packages/react-events/src/dom/Hover.js @@ -321,7 +321,7 @@ const HoverResponder: ReactDOMEventResponder = { if (isEmulatedMouseEvent(event, state)) { return; } - state.hoverTarget = event.currentTarget; + state.hoverTarget = event.responderTarget; state.ignoreEmulatedMouseEvents = true; dispatchHoverStartEvents(event, context, props, state); } diff --git a/packages/react-events/src/dom/Press.js b/packages/react-events/src/dom/Press.js index 75a85b9a46464..4f00373b26ed0 100644 --- a/packages/react-events/src/dom/Press.js +++ b/packages/react-events/src/dom/Press.js @@ -696,7 +696,7 @@ const PressResponder: ReactDOMEventResponder = { // We set these here, before the button check so we have this // data around for handling of the context menu state.pointerType = pointerType; - const pressTarget = (state.pressTarget = event.currentTarget); + const pressTarget = (state.pressTarget = event.responderTarget); if (isPointerEvent) { state.activePointerId = pointerId; } else if (isTouchEvent) { diff --git a/packages/react-events/src/dom/__tests__/Press-test.internal.js b/packages/react-events/src/dom/__tests__/Press-test.internal.js index 39037185fe65a..c849c88aad082 100644 --- a/packages/react-events/src/dom/__tests__/Press-test.internal.js +++ b/packages/react-events/src/dom/__tests__/Press-test.internal.js @@ -3119,6 +3119,34 @@ describe('Event responder: Press', () => { expect(pointerDownEvent).toHaveBeenCalledTimes(0); }); + it('has the correct press target when used with event hook', () => { + const ref = React.createRef(); + const onPress = jest.fn(); + const Component = () => { + React.unstable_useEvent(Press, {onPress}); + + return ( + + ); + }; + ReactDOM.render(, container); + + ref.current.dispatchEvent( + createEvent('pointerdown', {pointerType: 'mouse', button: 0}), + ); + ref.current.dispatchEvent( + createEvent('pointerup', {pointerType: 'mouse', button: 0}), + ); + expect(onPress).toHaveBeenCalledTimes(1); + expect(onPress).toHaveBeenCalledWith( + expect.objectContaining({target: ref.current}), + ); + }); + it('warns when stopPropagation is used in an event hook', () => { const ref = React.createRef(); const Component = () => { diff --git a/packages/shared/ReactDOMTypes.js b/packages/shared/ReactDOMTypes.js index 013ebed24d0bd..e570d0baab46d 100644 --- a/packages/shared/ReactDOMTypes.js +++ b/packages/shared/ReactDOMTypes.js @@ -24,12 +24,12 @@ export type PointerType = | 'trackpad'; export type ReactDOMResponderEvent = { - currentTarget: null | Element | Document, nativeEvent: AnyNativeEvent, passive: boolean, passiveSupported: boolean, pointerId: null | number, pointerType: PointerType, + responderTarget: null | Element | Document, target: Element | Document, type: string, };