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

Add container to event listener signature #18075

Merged
merged 1 commit into from
Feb 19, 2020
Merged

Add container to event listener signature #18075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions packages/legacy-events/ReactGenericBatching.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils';
let batchedUpdatesImpl = function(fn, bookkeeping) {
return fn(bookkeeping);
};
let discreteUpdatesImpl = function(fn, a, b, c) {
return fn(a, b, c);
let discreteUpdatesImpl = function(fn, a, b, c, d) {
return fn(a, b, c, d);
};
let flushDiscreteUpdatesImpl = function() {};
let batchedEventUpdatesImpl = batchedUpdatesImpl;
Expand Down Expand Up @@ -89,11 +89,11 @@ export function executeUserEventHandler(fn: any => void, value: any): void {
}
}

export function discreteUpdates(fn, a, b, c) {
export function discreteUpdates(fn, a, b, c, d) {
const prevIsInsideEventHandler = isInsideEventHandler;
isInsideEventHandler = true;
try {
return discreteUpdatesImpl(fn, a, b, c);
return discreteUpdatesImpl(fn, a, b, c, d);
} finally {
isInsideEventHandler = prevIsInsideEventHandler;
if (!isInsideEventHandler) {
Expand Down
53 changes: 45 additions & 8 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 @@ -119,6 +119,7 @@ export function addResponderEventSystemEvent(
null,
((topLevelType: any): DOMTopLevelEventType),
eventFlags,
document,
);
if (passiveBrowserEventsSupported) {
addEventCaptureListenerWithPassiveFlag(
Expand Down Expand Up @@ -149,7 +150,7 @@ export function removeActiveResponderEventSystemEvent(
}

function trapEventForPluginEventSystem(
element: Document | Element | Node,
container: Document | Element | Node,
topLevelType: DOMTopLevelEventType,
capture: boolean,
): void {
Expand All @@ -160,48 +161,74 @@ function trapEventForPluginEventSystem(
null,
topLevelType,
PLUGIN_EVENT_SYSTEM,
container,
);
break;
case UserBlockingEvent:
listener = dispatchUserBlockingUpdate.bind(
null,
topLevelType,
PLUGIN_EVENT_SYSTEM,
container,
);
break;
case ContinuousEvent:
default:
listener = dispatchEvent.bind(null, topLevelType, PLUGIN_EVENT_SYSTEM);
listener = dispatchEvent.bind(
null,
topLevelType,
PLUGIN_EVENT_SYSTEM,
container,
);
break;
}

const rawEventName = getRawEventName(topLevelType);
if (capture) {
addEventCaptureListener(element, rawEventName, listener);
addEventCaptureListener(container, rawEventName, listener);
} else {
addEventBubbleListener(element, rawEventName, listener);
addEventBubbleListener(container, rawEventName, listener);
}
}

function dispatchDiscreteEvent(topLevelType, eventSystemFlags, nativeEvent) {
function dispatchDiscreteEvent(
topLevelType,
eventSystemFlags,
container,
nativeEvent,
) {
flushDiscreteUpdatesIfNeeded(nativeEvent.timeStamp);
discreteUpdates(dispatchEvent, topLevelType, eventSystemFlags, nativeEvent);
discreteUpdates(
dispatchEvent,
topLevelType,
eventSystemFlags,
container,
nativeEvent,
);
}

function dispatchUserBlockingUpdate(
topLevelType,
eventSystemFlags,
container,
nativeEvent,
) {
runWithPriority(
UserBlockingPriority,
dispatchEvent.bind(null, topLevelType, eventSystemFlags, nativeEvent),
dispatchEvent.bind(
null,
topLevelType,
eventSystemFlags,
container,
nativeEvent,
),
);
}

export function dispatchEvent(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
container: Document | Element | Node,
nativeEvent: AnyNativeEvent,
): void {
if (!_enabled) {
Expand All @@ -216,6 +243,7 @@ export function dispatchEvent(
topLevelType,
eventSystemFlags,
nativeEvent,
container,
);
return;
}
Expand All @@ -224,6 +252,7 @@ export function dispatchEvent(
topLevelType,
eventSystemFlags,
nativeEvent,
container,
);

if (blockedOn === null) {
Expand All @@ -234,7 +263,13 @@ export function dispatchEvent(

if (isReplayableDiscreteEvent(topLevelType)) {
// This this to be replayed later once the target is available.
queueDiscreteEvent(blockedOn, topLevelType, eventSystemFlags, nativeEvent);
queueDiscreteEvent(
blockedOn,
topLevelType,
eventSystemFlags,
nativeEvent,
container,
);
return;
}

Expand All @@ -244,6 +279,7 @@ export function dispatchEvent(
topLevelType,
eventSystemFlags,
nativeEvent,
container,
)
) {
return;
Expand Down Expand Up @@ -289,6 +325,7 @@ export function attemptToDispatchEvent(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
nativeEvent: AnyNativeEvent,
container: Document | Element | Node,
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of the other signatures have it before nativeEvent so this was a bit confusing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess the ones below don't. It would be nice to preserve a consistent order because it's easy to mix them up. Hopefully Flow would catch that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flow can catch it, and it did, because I've hacked this file abount so much in the last few days and it's saved my ass a bunch :)

I can refine the order in a follow up though, you're right, consistency helps here.

): null | Container | SuspenseInstance {
// TODO: Warn if _enabled is false.

Expand Down
15 changes: 15 additions & 0 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 @@ -124,6 +124,7 @@ type QueuedReplayableEvent = {|
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
nativeEvent: AnyNativeEvent,
container: Document | Element | Node,
|};

let hasScheduledReplayAttempt = false;
Expand Down Expand Up @@ -252,12 +253,14 @@ function createQueuedReplayableEvent(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
nativeEvent: AnyNativeEvent,
container: Document | Element | Node,
): QueuedReplayableEvent {
return {
blockedOn,
topLevelType,
eventSystemFlags: eventSystemFlags | IS_REPLAYED,
nativeEvent,
container,
};
}

Expand All @@ -266,12 +269,14 @@ export function queueDiscreteEvent(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
nativeEvent: AnyNativeEvent,
container: Document | Element | Node,
): void {
const queuedEvent = createQueuedReplayableEvent(
blockedOn,
topLevelType,
eventSystemFlags,
nativeEvent,
container,
);
queuedDiscreteEvents.push(queuedEvent);
if (enableSelectiveHydration) {
Expand Down Expand Up @@ -339,6 +344,7 @@ function accumulateOrCreateContinuousQueuedReplayableEvent(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
nativeEvent: AnyNativeEvent,
container: Document | Element | Node,
): QueuedReplayableEvent {
if (
existingQueuedEvent === null ||
Expand All @@ -349,6 +355,7 @@ function accumulateOrCreateContinuousQueuedReplayableEvent(
topLevelType,
eventSystemFlags,
nativeEvent,
container,
);
if (blockedOn !== null) {
let fiber = getInstanceFromNode(blockedOn);
Expand All @@ -372,6 +379,7 @@ export function queueIfContinuousEvent(
topLevelType: DOMTopLevelEventType,
eventSystemFlags: EventSystemFlags,
nativeEvent: AnyNativeEvent,
container: Document | Element | Node,
): boolean {
// These set relatedTarget to null because the replayed event will be treated as if we
// moved from outside the window (no target) onto the target once it hydrates.
Expand All @@ -385,6 +393,7 @@ export function queueIfContinuousEvent(
topLevelType,
eventSystemFlags,
focusEvent,
container,
);
return true;
}
Expand All @@ -396,6 +405,7 @@ export function queueIfContinuousEvent(
topLevelType,
eventSystemFlags,
dragEvent,
container,
);
return true;
}
Expand All @@ -407,6 +417,7 @@ export function queueIfContinuousEvent(
topLevelType,
eventSystemFlags,
mouseEvent,
container,
);
return true;
}
Expand All @@ -421,6 +432,7 @@ export function queueIfContinuousEvent(
topLevelType,
eventSystemFlags,
pointerEvent,
container,
),
);
return true;
Expand All @@ -436,6 +448,7 @@ export function queueIfContinuousEvent(
topLevelType,
eventSystemFlags,
pointerEvent,
container,
),
);
return true;
Expand Down Expand Up @@ -512,6 +525,7 @@ function attemptReplayContinuousQueuedEvent(
queuedEvent.topLevelType,
queuedEvent.eventSystemFlags,
queuedEvent.nativeEvent,
queuedEvent.container,
);
if (nextBlockedOn !== null) {
// We're still blocked. Try again later.
Expand Down Expand Up @@ -554,6 +568,7 @@ function replayUnblockedEvents() {
nextDiscreteEvent.topLevelType,
nextDiscreteEvent.eventSystemFlags,
nextDiscreteEvent.nativeEvent,
nextDiscreteEvent.container,
);
if (nextBlockedOn !== null) {
// We're still blocked. Try again later.
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/test-utils/ReactTestUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ let hasWarnedAboutDeprecatedMockComponent = false;
*/
function simulateNativeEventOnNode(topLevelType, node, fakeNativeEvent) {
fakeNativeEvent.target = node;
dispatchEvent(topLevelType, PLUGIN_EVENT_SYSTEM, fakeNativeEvent);
dispatchEvent(topLevelType, PLUGIN_EVENT_SYSTEM, document, fakeNativeEvent);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,17 +1160,18 @@ export function batchedEventUpdates<A, R>(fn: A => R, a: A): R {
}
}

export function discreteUpdates<A, B, C, R>(
export function discreteUpdates<A, B, C, D, R>(
fn: (A, B, C) => R,
a: A,
b: B,
c: C,
d: D,
): R {
const prevExecutionContext = executionContext;
executionContext |= DiscreteEventContext;
try {
// Should this
return runWithPriority(UserBlockingPriority, fn.bind(null, a, b, c));
return runWithPriority(UserBlockingPriority, fn.bind(null, a, b, c, d));
} finally {
executionContext = prevExecutionContext;
if (executionContext === NoContext) {
Expand Down