-
Notifications
You must be signed in to change notification settings - Fork 49.9k
Refine event registration + event signatures #19244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ import { | |
| LEGACY_FB_SUPPORT, | ||
| IS_REPLAYED, | ||
| IS_TARGET_PHASE_ONLY, | ||
| IS_CAPTURE_PHASE, | ||
| } from './EventSystemFlags'; | ||
|
|
||
| import { | ||
|
|
@@ -301,9 +302,9 @@ export function listenToTopLevelEvent( | |
| target: EventTarget, | ||
| listenerMap: ElementListenerMap, | ||
| eventSystemFlags: EventSystemFlags, | ||
| capture: boolean, | ||
| passive?: boolean, | ||
| priority?: EventPriority, | ||
| capture?: boolean, | ||
| ): void { | ||
| // TOP_SELECTION_CHANGE needs to be attached to the document | ||
| // otherwise it won't capture incoming events that are only | ||
|
|
@@ -312,12 +313,10 @@ export function listenToTopLevelEvent( | |
| target = (target: any).ownerDocument || target; | ||
| listenerMap = getEventListenerMap(target); | ||
| } | ||
| capture = | ||
| capture === undefined ? capturePhaseEvents.has(topLevelType) : capture; | ||
| const listenerMapKey = getListenerMapKey(topLevelType, capture); | ||
| const listenerEntry: ElementListenerMapEntry | void = listenerMap.get( | ||
| const listenerEntry = ((listenerMap.get( | ||
| listenerMapKey, | ||
| ); | ||
| ): any): ElementListenerMapEntry | void); | ||
| const shouldUpgrade = shouldUpgradeListener(listenerEntry, passive); | ||
|
|
||
| // If the listener entry is empty or we should upgrade, then | ||
|
|
@@ -333,6 +332,9 @@ export function listenToTopLevelEvent( | |
| ((listenerEntry: any): ElementListenerMapEntry).listener, | ||
| ); | ||
| } | ||
| if (capture) { | ||
| eventSystemFlags |= IS_CAPTURE_PHASE; | ||
| } | ||
| const listener = addTrappedEventListener( | ||
| target, | ||
| topLevelType, | ||
|
|
@@ -346,20 +348,31 @@ export function listenToTopLevelEvent( | |
| } | ||
| } | ||
|
|
||
| export function listenToEvent( | ||
| registrationName: string, | ||
| export function listenToReactPropEvent( | ||
| reactPropEvent: string, | ||
| rootContainerElement: Element, | ||
| ): void { | ||
| const listenerMap = getEventListenerMap(rootContainerElement); | ||
| const dependencies = registrationNameDependencies[registrationName]; | ||
| // For optimization, let's check if we have the registration name | ||
| // on the rootContainerElement. | ||
| if (listenerMap.has(reactPropEvent)) { | ||
| return; | ||
| } | ||
| // Add the registration name to the map, so we can avoid processing | ||
| // this React prop event again. | ||
| listenerMap.set(reactPropEvent, null); | ||
| const dependencies = registrationNameDependencies[reactPropEvent]; | ||
|
|
||
| for (let i = 0; i < dependencies.length; i++) { | ||
| const dependency = dependencies[i]; | ||
| const capture = capturePhaseEvents.has(dependency); | ||
|
|
||
| listenToTopLevelEvent( | ||
| dependency, | ||
| rootContainerElement, | ||
| listenerMap, | ||
| PLUGIN_EVENT_SYSTEM, | ||
| capture, | ||
| ); | ||
| } | ||
| } | ||
|
|
@@ -892,10 +905,11 @@ export function accumulateEnterLeaveListeners( | |
| } | ||
| } | ||
|
|
||
| export function accumulateEventTargetListeners( | ||
| export function accumulateEventHandleTargetListeners( | ||
| dispatchQueue: DispatchQueue, | ||
| event: ReactSyntheticEvent, | ||
| currentTarget: EventTarget, | ||
| inCapturePhase: boolean, | ||
| ): void { | ||
| const capturePhase: DispatchQueueItemPhase = []; | ||
| const bubblePhase: DispatchQueueItemPhase = []; | ||
|
|
@@ -904,17 +918,16 @@ export function accumulateEventTargetListeners( | |
| if (eventListeners !== null) { | ||
| const listenersArr = Array.from(eventListeners); | ||
| const targetType = ((event.type: any): DOMTopLevelEventType); | ||
| const isCapturePhase = (event: any).eventPhase === 1; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a significant change. Instead of reading this from the event, we now pass it from outside based on whether it's in a list. Can you please explain why this is a refactoring rather than a semantic change? And what does it accomplish, exactly?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess there are two things here to note:
|
||
|
|
||
| for (let i = 0; i < listenersArr.length; i++) { | ||
| const listener = listenersArr[i]; | ||
| const {callback, capture, type} = listener; | ||
| if (type === targetType) { | ||
| if (isCapturePhase && capture) { | ||
| if (inCapturePhase && capture) { | ||
| capturePhase.push( | ||
| createDispatchQueueItemPhaseEntry(null, callback, currentTarget), | ||
| ); | ||
| } else if (!isCapturePhase && !capture) { | ||
| } else if (!inCapturePhase && !capture) { | ||
| bubblePhase.push( | ||
| createDispatchQueueItemPhaseEntry(null, callback, currentTarget), | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,11 +29,7 @@ import { | |
| } from '../../client/ReactDOMComponentTree'; | ||
| import {hasSelectionCapabilities} from '../../client/ReactInputSelection'; | ||
| import {DOCUMENT_NODE} from '../../shared/HTMLNodeType'; | ||
| import { | ||
| accumulateTwoPhaseListeners, | ||
| getListenerMapKey, | ||
| capturePhaseEvents, | ||
| } from '../DOMModernPluginEventSystem'; | ||
| import {accumulateTwoPhaseListeners} from '../DOMModernPluginEventSystem'; | ||
|
|
||
| const skipSelectionChangeEvent = | ||
| canUseDOM && 'documentMode' in document && document.documentMode <= 11; | ||
|
|
@@ -148,32 +144,6 @@ function constructSelectEvent(dispatchQueue, nativeEvent, nativeEventTarget) { | |
| } | ||
| } | ||
|
|
||
| function isListeningToEvents( | ||
| events: Array<string>, | ||
| mountAt: Document | Element, | ||
| ): boolean { | ||
| const listenerMap = getEventListenerMap(mountAt); | ||
| for (let i = 0; i < events.length; i++) { | ||
| const event = events[i]; | ||
| const capture = capturePhaseEvents.has(event); | ||
| const listenerMapKey = getListenerMapKey(event, capture); | ||
| if (!listenerMap.has(listenerMapKey)) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| function isListeningToEvent( | ||
| registrationName: string, | ||
| mountAt: Document | Element, | ||
| ): boolean { | ||
| const listenerMap = getEventListenerMap(mountAt); | ||
| const capture = capturePhaseEvents.has(registrationName); | ||
| const listenerMapKey = getListenerMapKey(registrationName, capture); | ||
| return listenerMap.has(listenerMapKey); | ||
| } | ||
|
|
||
| /** | ||
| * This plugin creates an `onSelect` event that normalizes select events | ||
| * across form elements. | ||
|
|
@@ -197,19 +167,17 @@ function extractEvents( | |
| eventSystemFlags, | ||
| targetContainer, | ||
| ) { | ||
| const doc = getEventTargetDocument(nativeEventTarget); | ||
| const eventListenerMap = getEventListenerMap(targetContainer); | ||
| // Track whether all listeners exists for this plugin. If none exist, we do | ||
| // not extract events. See #3639. | ||
| if ( | ||
| // We only listen to TOP_SELECTION_CHANGE on the document, never the | ||
| // root. | ||
| !isListeningToEvent(TOP_SELECTION_CHANGE, doc) || | ||
| // If we are handling TOP_SELECTION_CHANGE, then we don't need to | ||
| // check for the other dependencies, as TOP_SELECTION_CHANGE is only | ||
| // event attached from the onChange plugin and we don't expose an | ||
| // onSelectionChange event from React. | ||
| (topLevelType !== TOP_SELECTION_CHANGE && | ||
| !isListeningToEvents(rootTargetDependencies, targetContainer)) | ||
| topLevelType !== TOP_SELECTION_CHANGE && | ||
| !eventListenerMap.has('onSelect') && | ||
| !eventListenerMap.has('onSelectCapture') | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why this check is equivalent? It's not obvious to me.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than going through all the dependencies one by one, we can instead check if we've registered |
||
| ) { | ||
| return; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.