Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Decouple dispatching from attemptToDispatchEvent #22851

Merged
merged 2 commits into from
Dec 2, 2021
Merged

Decouple dispatching from attemptToDispatchEvent #22851

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bcc386
Decoupled dispatching from attemptToDispatchEvent
gaearon Dec 1, 2021
905ab99
Remove unnecessary field
gaearon Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 31 additions & 12 deletions packages/react-dom/src/events/ReactDOMEventListener.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,21 @@ export function dispatchEvent(
return;
}

let blockedOn = attemptToDispatchEvent(
let blockedOn = findInstanceBlockingEvent(
domEventName,
eventSystemFlags,
targetContainer,
nativeEvent,
);

if (return_shouldDispatch) {
dispatchEventForPluginEventSystem(
domEventName,
eventSystemFlags,
nativeEvent,
return_targetInst,
targetContainer,
);
}
if (blockedOn === null) {
// We successfully dispatched this event.
if (allowReplay) {
Expand Down Expand Up @@ -236,12 +244,21 @@ export function dispatchEvent(
if (fiber !== null) {
attemptSynchronousHydration(fiber);
}
const nextBlockedOn = attemptToDispatchEvent(
const nextBlockedOn = findInstanceBlockingEvent(
domEventName,
eventSystemFlags,
targetContainer,
nativeEvent,
);
if (return_shouldDispatch) {
dispatchEventForPluginEventSystem(
domEventName,
eventSystemFlags,
nativeEvent,
return_targetInst,
targetContainer,
);
}
if (nextBlockedOn === blockedOn) {
break;
}
Expand All @@ -264,13 +281,20 @@ export function dispatchEvent(
);
}

// Attempt dispatching an event. Returns a SuspenseInstance or Container if it's blocked.
export function attemptToDispatchEvent(
export let return_shouldDispatch = false;
export let return_targetInst = null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this leaks at module level but next event will clear it. So it's fine.


// Returns a SuspenseInstance or Container if it's blocked.
// The two fields above are conceptually part of the return value.
export function findInstanceBlockingEvent(
domEventName: DOMEventName,
eventSystemFlags: EventSystemFlags,
targetContainer: EventTarget,
nativeEvent: AnyNativeEvent,
): null | Container | SuspenseInstance {
return_shouldDispatch = false;
return_targetInst = null;

// TODO: Warn if _enabled is false.

const nativeEventTarget = getEventTarget(nativeEvent);
Expand Down Expand Up @@ -313,13 +337,8 @@ export function attemptToDispatchEvent(
}
}
}
dispatchEventForPluginEventSystem(
domEventName,
eventSystemFlags,
nativeEvent,
targetInst,
targetContainer,
);
return_targetInst = targetInst;
return_shouldDispatch = true;
// We're not blocked on anything.
return null;
}
Expand Down
31 changes: 27 additions & 4 deletions packages/react-dom/src/events/ReactDOMEventReplaying.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ import {
getContainerFromFiber,
getSuspenseInstanceFromFiber,
} from 'react-reconciler/src/ReactFiberTreeReflection';
import {attemptToDispatchEvent} from './ReactDOMEventListener';
import {
findInstanceBlockingEvent,
return_targetInst,
return_shouldDispatch,
} from './ReactDOMEventListener';
import {dispatchEventForPluginEventSystem} from './DOMPluginEventSystem';
import {
getInstanceFromNode,
getClosestInstanceFromNode,
Expand Down Expand Up @@ -389,7 +394,7 @@ export function queueIfContinuousEvent(
function attemptExplicitHydrationTarget(
queuedTarget: QueuedHydrationTarget,
): void {
// TODO: This function shares a lot of logic with attemptToDispatchEvent.
// TODO: This function shares a lot of logic with findInstanceBlockingEvent.
// Try to unify them. It's a bit tricky since it would require two return
// values.
const targetInst = getClosestInstanceFromNode(queuedTarget.target);
Expand Down Expand Up @@ -462,12 +467,21 @@ function attemptReplayContinuousQueuedEvent(
const targetContainers = queuedEvent.targetContainers;
while (targetContainers.length > 0) {
const targetContainer = targetContainers[0];
const nextBlockedOn = attemptToDispatchEvent(
const nextBlockedOn = findInstanceBlockingEvent(
queuedEvent.domEventName,
queuedEvent.eventSystemFlags,
targetContainer,
queuedEvent.nativeEvent,
);
if (return_shouldDispatch) {
dispatchEventForPluginEventSystem(
queuedEvent.domEventName,
queuedEvent.eventSystemFlags,
queuedEvent.nativeEvent,
return_targetInst,
targetContainer,
);
}
if (nextBlockedOn !== null) {
// We're still blocked. Try again later.
const fiber = getInstanceFromNode(nextBlockedOn);
Expand Down Expand Up @@ -512,12 +526,21 @@ function replayUnblockedEvents() {
const targetContainers = nextDiscreteEvent.targetContainers;
while (targetContainers.length > 0) {
const targetContainer = targetContainers[0];
const nextBlockedOn = attemptToDispatchEvent(
const nextBlockedOn = findInstanceBlockingEvent(
nextDiscreteEvent.domEventName,
nextDiscreteEvent.eventSystemFlags,
targetContainer,
nextDiscreteEvent.nativeEvent,
);
if (return_shouldDispatch) {
dispatchEventForPluginEventSystem(
nextDiscreteEvent.domEventName,
nextDiscreteEvent.eventSystemFlags,
nextDiscreteEvent.nativeEvent,
return_targetInst,
targetContainer,
);
}
if (nextBlockedOn !== null) {
// We're still blocked. Try again later.
nextDiscreteEvent.blockedOn = nextBlockedOn;
Expand Down