Skip to content

Commit

Permalink
React Event System: Refactor ElementListenerMap for upgrading (#18308)
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm authored Mar 18, 2020
1 parent fe1f79b commit 7d466bc
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 36 deletions.
15 changes: 10 additions & 5 deletions packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -1358,22 +1358,27 @@ export function listenToEventResponderEventTypes(
// existing passive event listener before we add the
// active event listener.
const passiveKey = targetEventType + '_passive';
const passiveListener = listenerMap.get(passiveKey);
if (passiveListener != null) {
const passiveItem = listenerMap.get(passiveKey);
if (passiveItem !== undefined) {
removeTrappedEventListener(
document,
targetEventType,
passiveListener,
(targetEventType: any),
true,
passiveItem.listener,
true,
);
listenerMap.delete(passiveKey);
}
}
const eventListener = addResponderEventSystemEvent(
document,
targetEventType,
isPassive,
);
listenerMap.set(eventKey, eventListener);
listenerMap.set(eventKey, {
passive: isPassive,
listener: eventListener,
});
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions packages/react-dom/src/events/DOMEventListenerMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@ const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
const elementListenerMap:
// $FlowFixMe Work around Flow bug
| WeakMap
| Map<EventTarget, Map<DOMTopLevelEventType | string, null | (any => void)>> = new PossiblyWeakMap();
| Map<EventTarget, ElementListenerMap> = new PossiblyWeakMap();

export type ElementListenerMap = Map<
DOMTopLevelEventType | string,
null | (any => void),
ElementListenerMapEntry,
>;

export type ElementListenerMapEntry = {
passive: void | boolean,
listener: any => void,
};

export function getListenerMapForElement(
target: EventTarget,
): ElementListenerMap {
Expand Down
37 changes: 21 additions & 16 deletions packages/react-dom/src/events/DOMLegacyEventPluginSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import type {AnyNativeEvent} from 'legacy-events/PluginModuleType';
import type {DOMTopLevelEventType} from 'legacy-events/TopLevelEventTypes';
import type {ElementListenerMap} from '../events/DOMEventListenerMap';
import type {EventSystemFlags} from 'legacy-events/EventSystemFlags';
import type {Fiber} from 'react-reconciler/src/ReactFiber';
import type {PluginModule} from 'legacy-events/PluginModuleType';
Expand Down Expand Up @@ -323,57 +324,61 @@ export function legacyListenToEvent(
export function legacyListenToTopLevelEvent(
topLevelType: DOMTopLevelEventType,
mountAt: Document | Element,
listenerMap: Map<DOMTopLevelEventType | string, null | (any => void)>,
listenerMap: ElementListenerMap,
): void {
if (!listenerMap.has(topLevelType)) {
switch (topLevelType) {
case TOP_SCROLL:
legacyTrapCapturedEvent(TOP_SCROLL, mountAt);
case TOP_SCROLL: {
legacyTrapCapturedEvent(TOP_SCROLL, mountAt, listenerMap);
break;
}
case TOP_FOCUS:
case TOP_BLUR:
legacyTrapCapturedEvent(TOP_FOCUS, mountAt);
legacyTrapCapturedEvent(TOP_BLUR, mountAt);
// We set the flag for a single dependency later in this function,
// but this ensures we mark both as attached rather than just one.
listenerMap.set(TOP_BLUR, null);
listenerMap.set(TOP_FOCUS, null);
legacyTrapCapturedEvent(TOP_FOCUS, mountAt, listenerMap);
legacyTrapCapturedEvent(TOP_BLUR, mountAt, listenerMap);
break;
case TOP_CANCEL:
case TOP_CLOSE:
case TOP_CLOSE: {
if (isEventSupported(getRawEventName(topLevelType))) {
legacyTrapCapturedEvent(topLevelType, mountAt);
legacyTrapCapturedEvent(topLevelType, mountAt, listenerMap);
}
break;
}
case TOP_INVALID:
case TOP_SUBMIT:
case TOP_RESET:
// We listen to them on the target DOM elements.
// Some of them bubble so we don't want them to fire twice.
break;
default:
default: {
// By default, listen on the top level to all non-media events.
// Media events don't bubble so adding the listener wouldn't do anything.
const isMediaEvent = mediaEventTypes.indexOf(topLevelType) !== -1;
if (!isMediaEvent) {
legacyTrapBubbledEvent(topLevelType, mountAt);
legacyTrapBubbledEvent(topLevelType, mountAt, listenerMap);
}
break;
}
}
listenerMap.set(topLevelType, null);
}
}

export function legacyTrapBubbledEvent(
topLevelType: DOMTopLevelEventType,
element: Document | Element,
listenerMap?: ElementListenerMap,
): void {
addTrappedEventListener(element, topLevelType, false);
const listener = addTrappedEventListener(element, topLevelType, false);
if (listenerMap) {
listenerMap.set(topLevelType, {passive: undefined, listener});
}
}

export function legacyTrapCapturedEvent(
topLevelType: DOMTopLevelEventType,
element: Document | Element,
listenerMap: ElementListenerMap,
): void {
addTrappedEventListener(element, topLevelType, true);
const listener = addTrappedEventListener(element, topLevelType, true);
listenerMap.set(topLevelType, {passive: undefined, listener});
}
49 changes: 42 additions & 7 deletions packages/react-dom/src/events/DOMModernPluginEventSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@

import type {AnyNativeEvent} from 'legacy-events/PluginModuleType';
import type {DOMTopLevelEventType} from 'legacy-events/TopLevelEventTypes';
import type {ElementListenerMap} from '../events/DOMEventListenerMap';
import type {
ElementListenerMap,
ElementListenerMapEntry,
} from '../events/DOMEventListenerMap';
import type {EventSystemFlags} from 'legacy-events/EventSystemFlags';
import type {EventPriority} from 'shared/ReactTypes';
import type {Fiber} from 'react-reconciler/src/ReactFiber';
Expand All @@ -24,7 +27,10 @@ import {plugins} from 'legacy-events/EventPluginRegistry';

import {HostRoot, HostPortal} from 'shared/ReactWorkTags';

import {addTrappedEventListener} from './ReactDOMEventListener';
import {
addTrappedEventListener,
removeTrappedEventListener,
} from './ReactDOMEventListener';
import getEventTarget from './getEventTarget';
import {getListenerMapForElement} from './DOMEventListenerMap';
import {
Expand Down Expand Up @@ -153,18 +159,47 @@ function dispatchEventsForPlugins(
}
}

function shouldUpgradeListener(
listenerEntry: void | ElementListenerMapEntry,
passive: void | boolean,
): boolean {
if (listenerEntry === undefined) {
return false;
}
// Upgrade from passive to active.
if (passive !== true && listenerEntry.passive) {
return true;
}
// Upgrade from default-active (browser default) to active.
if (passive === false && listenerEntry.passive === undefined) {
return true;
}
// Otherwise, do not upgrade
return false;
}

export function listenToTopLevelEvent(
topLevelType: DOMTopLevelEventType,
targetContainer: EventTarget,
listenerMap: ElementListenerMap,
passive?: boolean,
priority?: EventPriority,
): void {
// TODO: we need to know if the listenerMap previously was passive
// and to check if we need to upgrade to active. This will come in
// a useEvent follow up PR.
if (!listenerMap.has(topLevelType)) {
const listenerEntry = listenerMap.get(topLevelType);
const shouldUpgrade = shouldUpgradeListener(listenerEntry, passive);
if (listenerEntry === undefined || shouldUpgrade) {
const isCapturePhase = capturePhaseEvents.has(topLevelType);
// If we should upgrade, then we need to remove the existing trapped
// event listener for the target container.
if (shouldUpgrade) {
removeTrappedEventListener(
targetContainer,
topLevelType,
isCapturePhase,
((listenerEntry: any): ElementListenerMapEntry).listener,
((listenerEntry: any): ElementListenerMapEntry).passive,
);
}
const listener = addTrappedEventListener(
targetContainer,
topLevelType,
Expand All @@ -173,7 +208,7 @@ export function listenToTopLevelEvent(
passive,
priority,
);
listenerMap.set(topLevelType, listener);
listenerMap.set(topLevelType, {passive, listener});
}
}

Expand Down
12 changes: 7 additions & 5 deletions packages/react-dom/src/events/ReactDOMEventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,20 +255,22 @@ export function addTrappedEventListener(

export function removeTrappedEventListener(
targetContainer: EventTarget,
topLevelType: string,
topLevelType: DOMTopLevelEventType,
capture: boolean,
listener: any => void,
passive: boolean,
passive: void | boolean,
) {
if (listener.remove != null) {
listener.remove();
} else {
const rawEventName = getRawEventName(topLevelType);
if (passiveBrowserEventsSupported) {
targetContainer.removeEventListener(topLevelType, listener, {
capture: true,
targetContainer.removeEventListener(rawEventName, listener, {
capture,
passive,
});
} else {
targetContainer.removeEventListener(topLevelType, listener, true);
targetContainer.removeEventListener(rawEventName, listener, capture);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/events/ReactDOMEventReplaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ function trapReplayableEventForDocument(
topLevelTypeString,
false,
);
listenerMap.set(activeEventKey, listener);
listenerMap.set(activeEventKey, {passive: false, listener});
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1648,5 +1648,107 @@ describe('DOMModernPluginEventSystem', () => {
dispatchClickEvent(ref.current);
expect(log).toEqual([{counter: 1}]);
});

it('should correctly work for a basic "click" listener that upgrades', () => {
const clickEvent = jest.fn();
const buttonRef = React.createRef();
const button2Ref = React.createRef();

function Test2() {
const click = ReactDOM.unstable_useEvent('click', {
passive: false,
});

React.useEffect(() => {
click.setListener(button2Ref.current, clickEvent);
});

return <button ref={button2Ref}>Click me!</button>;
}

function Test({extra}) {
const click = ReactDOM.unstable_useEvent('click', {
passive: true,
});

React.useEffect(() => {
click.setListener(buttonRef.current, clickEvent);
});

return (
<>
<button ref={buttonRef}>Click me!</button>
{extra && <Test2 />}
</>
);
}

ReactDOM.render(<Test />, container);
Scheduler.unstable_flushAll();

let button = buttonRef.current;
dispatchClickEvent(button);
expect(clickEvent).toHaveBeenCalledTimes(1);

ReactDOM.render(<Test extra={true} />, container);
Scheduler.unstable_flushAll();

clickEvent.mockClear();

button = button2Ref.current;
dispatchClickEvent(button);
expect(clickEvent).toHaveBeenCalledTimes(1);
});

it('should correctly work for a basic "click" listener that upgrades #2', () => {
const clickEvent = jest.fn();
const buttonRef = React.createRef();
const button2Ref = React.createRef();

function Test2() {
const click = ReactDOM.unstable_useEvent('click', {
passive: false,
});

React.useEffect(() => {
click.setListener(button2Ref.current, clickEvent);
});

return <button ref={button2Ref}>Click me!</button>;
}

function Test({extra}) {
const click = ReactDOM.unstable_useEvent('click', {
passive: undefined,
});

React.useEffect(() => {
click.setListener(buttonRef.current, clickEvent);
});

return (
<>
<button ref={buttonRef}>Click me!</button>
{extra && <Test2 />}
</>
);
}

ReactDOM.render(<Test />, container);
Scheduler.unstable_flushAll();

let button = buttonRef.current;
dispatchClickEvent(button);
expect(clickEvent).toHaveBeenCalledTimes(1);

ReactDOM.render(<Test extra={true} />, container);
Scheduler.unstable_flushAll();

clickEvent.mockClear();

button = button2Ref.current;
dispatchClickEvent(button);
expect(clickEvent).toHaveBeenCalledTimes(1);
});
});
});

0 comments on commit 7d466bc

Please sign in to comment.