Bail-out of attaching non-delegated listeners (#19466)
* Bail-out of attaching non-delegated listeners Revise comment * Fix tests/add tests * Add onInvalid test
Dominic Gannaway committed
Jul 27, 2020 at 17:47 UTC
144297165b5cad384485f4738ac6740e75bd5ae0
2 files changed
+79
-29
packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
+67
-29
@@ -348,12 +348,7 @@ describe('ReactDOMEventListener', () => {
348
bubbles: false,
349
}),
350
);
351
- // As of the modern event system refactor, we now support
352
- // this on <img>. The reason for this, is because we allow
353
- // events to be attached to nodes regardless of if they
354
- // necessary support them. This is a strange test, as this
355
- // would never occur from normal browser behavior.
356
- expect(handleImgLoadStart).toHaveBeenCalledTimes(1);
351
+ expect(handleImgLoadStart).toHaveBeenCalledTimes(0);
352
353
videoRef.current.dispatchEvent(
354
new ProgressEvent('loadstart', {
@@ -535,36 +530,42 @@ describe('ReactDOMEventListener', () => {
530
}
531
});
532
538
- it('should bubble non-native bubbling events', () => {
533
+ it('should bubble non-native bubbling toggle events', () => {
534
const container = document.createElement('div');
535
const ref = React.createRef();
541
- const onPlay = jest.fn();
542
- const onCancel = jest.fn();
543
- const onClose = jest.fn();
536
const onToggle = jest.fn();
537
document.body.appendChild(container);
538
try {
539
ReactDOM.render(
548
- <div
549
- onPlay={onPlay}
550
- onCancel={onCancel}
551
- onClose={onClose}
552
- onToggle={onToggle}>
553
- <div
554
- ref={ref}
555
- onPlay={onPlay}
556
- onCancel={onCancel}
557
- onClose={onClose}
558
- onToggle={onToggle}
559
- />
540
+ <div onToggle={onToggle}>
541
+ <details ref={ref} onToggle={onToggle} />
542
</div>,
543
container,
544
);
545
ref.current.dispatchEvent(
564
- new Event('play', {
546
+ new Event('toggle', {
547
bubbles: false,
548
}),
549
);
550
+ expect(onToggle).toHaveBeenCalledTimes(2);
551
+ } finally {
552
+ document.body.removeChild(container);
553
+ }
554
+ });
555
+
556
+ it('should bubble non-native bubbling cancel/close events', () => {
557
+ const container = document.createElement('div');
558
+ const ref = React.createRef();
559
+ const onCancel = jest.fn();
560
+ const onClose = jest.fn();
561
+ document.body.appendChild(container);
562
+ try {
563
+ ReactDOM.render(
564
+ <div onCancel={onCancel} onClose={onClose}>
565
+ <dialog ref={ref} onCancel={onCancel} onClose={onClose} />
566
+ </div>,
567
+ container,
568
+ );
569
ref.current.dispatchEvent(
570
new Event('cancel', {
571
bubbles: false,
@@ -575,17 +576,54 @@ describe('ReactDOMEventListener', () => {
576
bubbles: false,
577
}),
578
);
579
+ expect(onCancel).toHaveBeenCalledTimes(2);
580
+ expect(onClose).toHaveBeenCalledTimes(2);
581
+ } finally {
582
+ document.body.removeChild(container);
583
+ }
584
+ });
585
+
586
+ it('should bubble non-native bubbling media events events', () => {
587
+ const container = document.createElement('div');
588
+ const ref = React.createRef();
589
+ const onPlay = jest.fn();
590
+ document.body.appendChild(container);
591
+ try {
592
+ ReactDOM.render(
593
+ <div onPlay={onPlay}>
594
+ <video ref={ref} onPlay={onPlay} />
595
+ </div>,
596
+ container,
597
+ );
598
ref.current.dispatchEvent(
579
- new Event('toggle', {
599
+ new Event('play', {
600
bubbles: false,
601
}),
602
);
583
- // Regression test: ensure we still emulate bubbling with non-bubbling
584
- // media
603
expect(onPlay).toHaveBeenCalledTimes(2);
586
- expect(onCancel).toHaveBeenCalledTimes(2);
587
- expect(onClose).toHaveBeenCalledTimes(2);
588
- expect(onToggle).toHaveBeenCalledTimes(2);
604
+ } finally {
605
+ document.body.removeChild(container);
606
+ }
607
+ });
608
+
609
+ it('should bubble non-native bubbling invalid events', () => {
610
+ const container = document.createElement('div');
611
+ const ref = React.createRef();
612
+ const onInvalid = jest.fn();
613
+ document.body.appendChild(container);
614
+ try {
615
+ ReactDOM.render(
616
+ <form onInvalid={onInvalid}>
617
+ <input ref={ref} onInvalid={onInvalid} />
618
+ </form>,
619
+ container,
620
+ );
621
+ ref.current.dispatchEvent(
622
+ new Event('invalid', {
623
+ bubbles: false,
624
+ }),
625
+ );
626
+ expect(onInvalid).toHaveBeenCalledTimes(2);
627
} finally {
628
document.body.removeChild(container);
629
}
packages/react-dom/src/events/DOMPluginEventSystem.js
+12
@@ -384,6 +384,18 @@ export function listenToNativeEvent(
384
!isCapturePhaseListener &&
385
nonDelegatedEvents.has(topLevelType)
386
) {
387
+ // For all non-delegated events, apart from scroll, we attach
388
+ // their event listeners to the respective elements that their
389
+ // events fire on. That means we can skip this step, as event
390
+ // listener has already been added previously. However, we
391
+ // special case the scroll event because the reality is that any
392
+ // element can scroll.
393
+ // TODO: ideally, we'd eventually apply the same logic to all
394
+ // events from the nonDelegatedEvents list. Then we can remove
395
+ // this special case and use the same logic for all events.
396
+ if (topLevelType !== TOP_SCROLL) {
397
+ return;
398
+ }
399
eventSystemFlags |= IS_NON_DELEGATED;
400
target = targetElement;
401
}