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.

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

[react-events] Various core tweaks for event responder system #16654

Merged
merged 2 commits into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
65 changes: 47 additions & 18 deletions packages/react-dom/src/events/DOMEventResponderSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
PASSIVE_NOT_SUPPORTED,
} from 'legacy-events/EventSystemFlags';
import type {AnyNativeEvent} from 'legacy-events/PluginModuleType';
import {HostComponent} from 'shared/ReactWorkTags';
import {HostComponent, ScopeComponent} from 'shared/ReactWorkTags';
import type {EventPriority} from 'shared/ReactTypes';
import type {
ReactDOMEventResponder,
Expand Down Expand Up @@ -66,6 +66,7 @@ type ResponderTimer = {|
instance: ReactDOMEventResponderInstance,
func: () => void,
id: number,
targetFiber: Fiber | null,
timeStamp: number,
|};

Expand All @@ -80,6 +81,7 @@ let currentTimers = new Map();
let currentInstance: null | ReactDOMEventResponderInstance = null;
let currentTimerIDCounter = 0;
let currentDocument: null | Document = null;
let currentTargetFiber: null | Fiber = null;

const eventResponderContext: ReactDOMResponderContext = {
dispatchEvent(
Expand Down Expand Up @@ -158,16 +160,20 @@ const eventResponderContext: ReactDOMResponderContext = {
validateResponderContext();
const childFiber = getClosestInstanceFromNode(childTarget);
const parentFiber = getClosestInstanceFromNode(parentTarget);
const parentAlternateFiber = parentFiber.alternate;

let node = childFiber;
while (node !== null) {
if (node === parentFiber || node === parentAlternateFiber) {
return true;
if (childFiber != null && parentFiber != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the fix taken from the other PR.

const parentAlternateFiber = parentFiber.alternate;
let node = childFiber;
while (node !== null) {
if (node === parentFiber || node === parentAlternateFiber) {
return true;
}
node = node.return;
}
node = node.return;
return false;
}
return false;
// Fallback to DOM APIs
return parentTarget.contains(childTarget);
},
addRootEventTypes(rootEventTypes: Array<string>): void {
validateResponderContext();
Expand Down Expand Up @@ -221,6 +227,7 @@ const eventResponderContext: ReactDOMResponderContext = {
instance: ((currentInstance: any): ReactDOMEventResponderInstance),
func,
id: timerId,
targetFiber: currentTargetFiber,
timeStamp: currentTimeStamp,
});
activeTimeouts.set(timerId, timeout);
Expand Down Expand Up @@ -260,6 +267,24 @@ const eventResponderContext: ReactDOMResponderContext = {
return false;
},
enqueueStateRestore,
getCurrentTarget(): Element | null {
validateResponderContext();
const responderFiber = ((currentInstance: any): ReactDOMEventResponderInstance)
.fiber;
let fiber = currentTargetFiber;
let currentTarget = null;

while (fiber !== null) {
if (fiber.tag === HostComponent) {
currentTarget = fiber.stateNode;
}
if (fiber === responderFiber || fiber.alternate === responderFiber) {
break;
}
fiber = fiber.return;
}
return currentTarget;
},
};

function validateEventValue(eventValue: any): void {
Expand Down Expand Up @@ -317,7 +342,8 @@ function doesFiberHaveResponder(
fiber: Fiber,
responder: ReactDOMEventResponder,
): boolean {
if (fiber.tag === HostComponent) {
const tag = fiber.tag;
if (tag === HostComponent || tag === ScopeComponent) {
const dependencies = fiber.dependencies;
if (dependencies !== null) {
const respondersMap = dependencies.responders;
Expand All @@ -341,8 +367,9 @@ function processTimers(
try {
batchedEventUpdates(() => {
for (let i = 0; i < timersArr.length; i++) {
const {instance, func, id, timeStamp} = timersArr[i];
const {instance, func, id, timeStamp, targetFiber} = timersArr[i];
currentInstance = instance;
currentTargetFiber = targetFiber;
currentTimeStamp = timeStamp + delay;
try {
func();
Expand All @@ -355,6 +382,7 @@ function processTimers(
currentTimers = null;
currentInstance = null;
currentTimeStamp = 0;
currentTargetFiber = null;
}
}

Expand Down Expand Up @@ -386,7 +414,6 @@ function createDOMResponderEvent(
passiveSupported,
pointerId,
pointerType: eventPointerType,
responderTarget: null,
target: nativeEventTarget,
type: topLevelType,
};
Expand Down Expand Up @@ -443,13 +470,16 @@ function traverseAndHandleEventResponderInstances(
let node = targetFiber;
while (node !== null) {
const {dependencies, tag} = node;
if (tag === HostComponent && dependencies !== null) {
if (
(tag === HostComponent || tag === ScopeComponent) &&
dependencies !== null
) {
const respondersMap = dependencies.responders;
if (respondersMap !== null) {
const responderInstances = Array.from(respondersMap.values());
for (let i = 0, length = responderInstances.length; i < length; i++) {
const responderInstance = responderInstances[i];
const {props, responder, state, target} = responderInstance;
const {props, responder, state} = responderInstance;
if (
!visitedResponders.has(responder) &&
validateResponderTargetEventTypes(eventType, responder)
Expand All @@ -458,9 +488,6 @@ function traverseAndHandleEventResponderInstances(
const onEvent = responder.onEvent;
if (onEvent !== null) {
currentInstance = responderInstance;
responderEvent.responderTarget = ((target: any):
| Element
| Document);
onEvent(responderEvent, eventResponderContext, props, state);
}
}
Expand All @@ -478,11 +505,10 @@ function traverseAndHandleEventResponderInstances(

for (let i = 0; i < responderInstances.length; i++) {
const responderInstance = responderInstances[i];
const {props, responder, state, target} = responderInstance;
const {props, responder, state} = responderInstance;
const onRootEvent = responder.onRootEvent;
if (onRootEvent !== null) {
currentInstance = responderInstance;
responderEvent.responderTarget = ((target: any): Element | Document);
onRootEvent(responderEvent, eventResponderContext, props, state);
}
}
Expand Down Expand Up @@ -562,7 +588,9 @@ export function dispatchEventForResponderEventSystem(
const previousTimers = currentTimers;
const previousTimeStamp = currentTimeStamp;
const previousDocument = currentDocument;
const previousTargetFiber = currentTargetFiber;
currentTimers = null;
currentTargetFiber = targetFiber;
// nodeType 9 is DOCUMENT_NODE
currentDocument =
(nativeEventTarget: any).nodeType === 9
Expand All @@ -585,6 +613,7 @@ export function dispatchEventForResponderEventSystem(
currentInstance = previousInstance;
currentTimeStamp = previousTimeStamp;
currentDocument = previousDocument;
currentTargetFiber = previousTargetFiber;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ describe('DOMEventResponderSystem', () => {
const obj = {
counter,
timeStamp: context.getTimeStamp(),
target: event.responderTarget,
target: context.getCurrentTarget(),
type: 'click-test',
};
context.dispatchEvent(obj, props.onClick, DiscreteEvent);
Expand Down
1 change: 0 additions & 1 deletion packages/react-events/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type ResponderEventType = string;

type ResponderEvent = {|
nativeEvent: any,
responderTarget: Element | Document,
target: Element | Document,
pointerType: string,
type: string,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-events/src/dom/Focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ const focusResponderImpl = {

switch (type) {
case 'focus': {
state.focusTarget = event.responderTarget;
state.focusTarget = context.getCurrentTarget();
// Limit focus events to the direct child of the event component.
// Browser focus is not expected to bubble.
if (!state.isFocused && state.focusTarget === target) {
Expand Down Expand Up @@ -427,7 +427,7 @@ const focusWithinResponderImpl = {

switch (type) {
case 'focus': {
state.focusTarget = event.responderTarget;
state.focusTarget = context.getCurrentTarget();
// Limit focus events to the direct child of the event component.
// Browser focus is not expected to bubble.
if (!state.isFocused) {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-events/src/dom/Hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ const hoverResponderImpl = {
// START
case 'pointerover': {
if (!state.isHovered && pointerType !== 'touch') {
state.hoverTarget = event.responderTarget;
state.hoverTarget = context.getCurrentTarget();
dispatchHoverStartEvents(event, context, props, state);
}
break;
Expand Down Expand Up @@ -295,7 +295,7 @@ const hoverResponderFallbackImpl = {
// START
case 'mouseover': {
if (!state.isHovered && !state.ignoreEmulatedMouseEvents) {
state.hoverTarget = event.responderTarget;
state.hoverTarget = context.getCurrentTarget();
dispatchHoverStartEvents(event, context, props, state);
}
break;
Expand Down
11 changes: 6 additions & 5 deletions packages/react-events/src/dom/Input.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,30 +179,31 @@ const inputResponderImpl = {
context: ReactDOMResponderContext,
props: InputResponderProps,
): void {
const {responderTarget, type, target} = event;
const {type, target} = event;

if (props.disabled) {
return;
}
if (target !== responderTarget || responderTarget === null) {
const currentTarget = context.getCurrentTarget();
if (target !== currentTarget || currentTarget === null) {
return;
}
switch (type) {
default: {
if (shouldUseChangeEvent(target) && type === 'change') {
dispatchBothChangeEvents(event, context, props, responderTarget);
dispatchBothChangeEvents(event, context, props, currentTarget);
} else if (
isTextInputElement(target) &&
(type === 'input' || type === 'change') &&
updateValueIfChanged(target)
) {
dispatchBothChangeEvents(event, context, props, responderTarget);
dispatchBothChangeEvents(event, context, props, currentTarget);
} else if (
isCheckable(target) &&
type === 'click' &&
updateValueIfChanged(target)
) {
dispatchBothChangeEvents(event, context, props, responderTarget);
dispatchBothChangeEvents(event, context, props, currentTarget);
}
break;
}
Expand Down
8 changes: 2 additions & 6 deletions packages/react-events/src/dom/Keyboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ function createKeyboardEvent(
event: ReactDOMResponderEvent,
context: ReactDOMResponderContext,
type: KeyboardEventType,
target: Document | Element,
defaultPrevented: boolean,
): KeyboardEvent {
const nativeEvent = (event: any).nativeEvent;
Expand All @@ -144,6 +143,7 @@ function createKeyboardEvent(
repeat,
shiftKey,
} = nativeEvent;
const target = ((context.getCurrentTarget(): any): Element);

return {
altKey,
Expand All @@ -166,14 +166,12 @@ function dispatchKeyboardEvent(
listener: KeyboardEvent => void,
context: ReactDOMResponderContext,
type: KeyboardEventType,
target: Element | Document,
defaultPrevented: boolean,
): void {
const syntheticEvent = createKeyboardEvent(
event,
context,
type,
target,
defaultPrevented,
);
context.dispatchEvent(syntheticEvent, listener, DiscreteEvent);
Expand All @@ -186,7 +184,7 @@ const keyboardResponderImpl = {
context: ReactDOMResponderContext,
props: KeyboardProps,
): void {
const {responderTarget, type} = event;
const {type} = event;
const nativeEvent: any = event.nativeEvent;

if (props.disabled) {
Expand Down Expand Up @@ -227,7 +225,6 @@ const keyboardResponderImpl = {
onKeyDown,
context,
'keydown',
((responderTarget: any): Element | Document),
defaultPrevented,
);
}
Expand All @@ -239,7 +236,6 @@ const keyboardResponderImpl = {
onKeyUp,
context,
'keyup',
((responderTarget: any): Element | Document),
defaultPrevented,
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-events/src/dom/Press.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ const pressResponderImpl = {
// 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.responderTarget);
const pressTarget = (state.pressTarget = context.getCurrentTarget());
if (isPointerEvent) {
state.activePointerId = pointerId;
} else if (isTouchEvent) {
Expand Down Expand Up @@ -634,7 +634,7 @@ const pressResponderImpl = {

if (isFunction(onPress) && isScreenReaderVirtualClick(nativeEvent)) {
state.pointerType = 'keyboard';
state.pressTarget = event.responderTarget;
state.pressTarget = context.getCurrentTarget();
const preventDefault = props.preventDefault;

if (preventDefault !== false) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-events/src/rn/Press.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ const pressResponderImpl = {
if (type === 'topTouchStart') {
if (!state.isPressed) {
state.pointerType = 'touch';
const pressTarget = (state.pressTarget = event.responderTarget);
const pressTarget = (state.pressTarget = context.getCurrentTarget());
const touchEvent = getTouchFromPressEvent(nativeEvent);
if (touchEvent === null) {
return;
Expand Down
Loading