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

Bail-out of attaching non-delegated listeners #19466

Merged
merged 3 commits into from Jul 27, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 67 additions & 29 deletions packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
Expand Up @@ -348,12 +348,7 @@ describe('ReactDOMEventListener', () => {
bubbles: false,
}),
);
// As of the modern event system refactor, we now support
// this on <img>. The reason for this, is because we allow
// events to be attached to nodes regardless of if they
// necessary support them. This is a strange test, as this
// would never occur from normal browser behavior.
expect(handleImgLoadStart).toHaveBeenCalledTimes(1);
expect(handleImgLoadStart).toHaveBeenCalledTimes(0);

videoRef.current.dispatchEvent(
new ProgressEvent('loadstart', {
Expand Down Expand Up @@ -535,36 +530,42 @@ describe('ReactDOMEventListener', () => {
}
});

it('should bubble non-native bubbling events', () => {
it('should bubble non-native bubbling toggle events', () => {
const container = document.createElement('div');
const ref = React.createRef();
const onPlay = jest.fn();
const onCancel = jest.fn();
const onClose = jest.fn();
const onToggle = jest.fn();
document.body.appendChild(container);
try {
ReactDOM.render(
<div
onPlay={onPlay}
onCancel={onCancel}
onClose={onClose}
onToggle={onToggle}>
<div
ref={ref}
onPlay={onPlay}
onCancel={onCancel}
onClose={onClose}
onToggle={onToggle}
/>
<div onToggle={onToggle}>
<details ref={ref} onToggle={onToggle} />
</div>,
container,
);
ref.current.dispatchEvent(
new Event('play', {
new Event('toggle', {
bubbles: false,
}),
);
expect(onToggle).toHaveBeenCalledTimes(2);
} finally {
document.body.removeChild(container);
}
});

it('should bubble non-native bubbling cancel/close events', () => {
const container = document.createElement('div');
const ref = React.createRef();
const onCancel = jest.fn();
const onClose = jest.fn();
document.body.appendChild(container);
try {
ReactDOM.render(
<div onCancel={onCancel} onClose={onClose}>
<dialog ref={ref} onCancel={onCancel} onClose={onClose} />
</div>,
container,
);
ref.current.dispatchEvent(
new Event('cancel', {
bubbles: false,
Expand All @@ -575,17 +576,54 @@ describe('ReactDOMEventListener', () => {
bubbles: false,
}),
);
expect(onCancel).toHaveBeenCalledTimes(2);
expect(onClose).toHaveBeenCalledTimes(2);
} finally {
document.body.removeChild(container);
}
});

it('should bubble non-native bubbling media events events', () => {
const container = document.createElement('div');
const ref = React.createRef();
const onPlay = jest.fn();
document.body.appendChild(container);
try {
ReactDOM.render(
<div onPlay={onPlay}>
<video ref={ref} onPlay={onPlay} />
</div>,
container,
);
ref.current.dispatchEvent(
new Event('toggle', {
new Event('play', {
bubbles: false,
}),
);
// Regression test: ensure we still emulate bubbling with non-bubbling
// media
expect(onPlay).toHaveBeenCalledTimes(2);
expect(onCancel).toHaveBeenCalledTimes(2);
expect(onClose).toHaveBeenCalledTimes(2);
expect(onToggle).toHaveBeenCalledTimes(2);
} finally {
document.body.removeChild(container);
}
});

it('should bubble non-native bubbling invalid events', () => {
const container = document.createElement('div');
const ref = React.createRef();
const onInvalid = jest.fn();
document.body.appendChild(container);
try {
ReactDOM.render(
<form onInvalid={onInvalid}>
<input ref={ref} onInvalid={onInvalid} />
</form>,
container,
);
ref.current.dispatchEvent(
new Event('invalid', {
bubbles: false,
}),
);
expect(onInvalid).toHaveBeenCalledTimes(2);
} finally {
document.body.removeChild(container);
}
Expand Down
12 changes: 12 additions & 0 deletions packages/react-dom/src/events/DOMPluginEventSystem.js
Expand Up @@ -384,6 +384,18 @@ export function listenToNativeEvent(
!isCapturePhaseListener &&
nonDelegatedEvents.has(topLevelType)
) {
// For all non-delegated events, apart from scroll, we attach
// their event listeners to the respective elements that their
// events fire on. That means we can skip this step, as event
// listener has already been added previously. However, we
// special case the scroll event because the reality is that any
// element can scroll.
// TODO: ideally, we'd eventually apply the same logic to all
// events from the nonDelegatedEvents list. Then we can remove
// this special case and use the same logic for all events.
if (topLevelType !== TOP_SCROLL) {
return;
}
eventSystemFlags |= IS_NON_DELEGATED;
target = targetElement;
}
Expand Down