Don't emulate bubbling of the scroll event (#19464)
* Don't emulate bubbling of the scroll event * Put behind a flag
Dan Abramov committed
Jul 27, 2020 at 17:33 UTC
06d104e8ec89df4bc5176e014c83e8b6393e555f
11 files changed
+132
-16
packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
+105
-9
@@ -539,7 +539,6 @@ describe('ReactDOMEventListener', () => {
539
const container = document.createElement('div');
540
const ref = React.createRef();
541
const onPlay = jest.fn();
542
- const onScroll = jest.fn();
542
const onCancel = jest.fn();
543
const onClose = jest.fn();
544
const onToggle = jest.fn();
@@ -548,14 +547,12 @@ describe('ReactDOMEventListener', () => {
547
ReactDOM.render(
548
<div
549
onPlay={onPlay}
551
- onScroll={onScroll}
550
onCancel={onCancel}
551
onClose={onClose}
552
onToggle={onToggle}>
553
<div
554
ref={ref}
555
onPlay={onPlay}
558
- onScroll={onScroll}
556
onCancel={onCancel}
557
onClose={onClose}
558
onToggle={onToggle}
@@ -568,11 +565,6 @@ describe('ReactDOMEventListener', () => {
565
bubbles: false,
566
}),
567
);
571
- ref.current.dispatchEvent(
572
- new Event('scroll', {
573
- bubbles: false,
574
- }),
575
- );
568
ref.current.dispatchEvent(
569
new Event('cancel', {
570
bubbles: false,
@@ -591,7 +583,6 @@ describe('ReactDOMEventListener', () => {
583
// Regression test: ensure we still emulate bubbling with non-bubbling
584
// media
585
expect(onPlay).toHaveBeenCalledTimes(2);
594
- expect(onScroll).toHaveBeenCalledTimes(2);
586
expect(onCancel).toHaveBeenCalledTimes(2);
587
expect(onClose).toHaveBeenCalledTimes(2);
588
expect(onToggle).toHaveBeenCalledTimes(2);
@@ -643,4 +634,109 @@ describe('ReactDOMEventListener', () => {
634
document.body.removeChild(container);
635
}
636
});
637
+
638
+ // We're moving towards aligning more closely with the browser.
639
+ // Currently we emulate bubbling for all non-bubbling events except scroll.
640
+ // We may expand this list in the future, removing emulated bubbling altogether.
641
+ it('should not emulate bubbling of scroll events', () => {
642
+ const container = document.createElement('div');
643
+ const ref = React.createRef();
644
+ const log = [];
645
+ const onScroll = jest.fn(e =>
646
+ log.push(['bubble', e.currentTarget.className]),
647
+ );
648
+ const onScrollCapture = jest.fn(e =>
649
+ log.push(['capture', e.currentTarget.className]),
650
+ );
651
+ document.body.appendChild(container);
652
+ try {
653
+ ReactDOM.render(
654
+ <div
655
+ className="grand"
656
+ onScroll={onScroll}
657
+ onScrollCapture={onScrollCapture}>
658
+ <div
659
+ className="parent"
660
+ onScroll={onScroll}
661
+ onScrollCapture={onScrollCapture}>
662
+ <div
663
+ className="child"
664
+ onScroll={onScroll}
665
+ onScrollCapture={onScrollCapture}
666
+ ref={ref}
667
+ />
668
+ </div>
669
+ </div>,
670
+ container,
671
+ );
672
+ ref.current.dispatchEvent(
673
+ new Event('scroll', {
674
+ bubbles: false,
675
+ }),
676
+ );
677
+ if (gate(flags => flags.disableOnScrollBubbling)) {
678
+ expect(log).toEqual([
679
+ ['capture', 'grand'],
680
+ ['capture', 'parent'],
681
+ ['capture', 'child'],
682
+ ['bubble', 'child'],
683
+ ]);
684
+ } else {
685
+ expect(log).toEqual([
686
+ ['capture', 'grand'],
687
+ ['capture', 'parent'],
688
+ ['capture', 'child'],
689
+ ['bubble', 'child'],
690
+ ['bubble', 'parent'],
691
+ ['bubble', 'grand'],
692
+ ]);
693
+ }
694
+ } finally {
695
+ document.body.removeChild(container);
696
+ }
697
+ });
698
+
699
+ // We're moving towards aligning more closely with the browser.
700
+ // Currently we emulate bubbling for all non-bubbling events except scroll.
701
+ // We may expand this list in the future, removing emulated bubbling altogether.
702
+ it('should not emulate bubbling of scroll events (no own handler)', () => {
703
+ const container = document.createElement('div');
704
+ const ref = React.createRef();
705
+ const log = [];
706
+ const onScroll = jest.fn(e =>
707
+ log.push(['bubble', e.currentTarget.className]),
708
+ );
709
+ const onScrollCapture = jest.fn(e =>
710
+ log.push(['capture', e.currentTarget.className]),
711
+ );
712
+ document.body.appendChild(container);
713
+ try {
714
+ ReactDOM.render(
715
+ <div
716
+ className="grand"
717
+ onScroll={onScroll}
718
+ onScrollCapture={onScrollCapture}>
719
+ <div
720
+ className="parent"
721
+ onScroll={onScroll}
722
+ onScrollCapture={onScrollCapture}>
723
+ {/* Intentionally no handler on the child: */}
724
+ <div className="child" ref={ref} />
725
+ </div>
726
+ </div>,
727
+ container,
728
+ );
729
+ ref.current.dispatchEvent(
730
+ new Event('scroll', {
731
+ bubbles: false,
732
+ }),
733
+ );
734
+ expect(log).toEqual([
735
+ ['capture', 'grand'],
736
+ ['capture', 'parent'],
737
+ ]);
738
+ } finally {
739
+ document.body.removeChild(container);
740
+ }
741
+ });
742
});
packages/react-dom/src/events/plugins/SimpleEventPlugin.js
+17
-7
@@ -42,7 +42,10 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../EventSystemFlags';
42
import getEventCharCode from '../getEventCharCode';
43
import {IS_CAPTURE_PHASE} from '../EventSystemFlags';
44
45
-import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';
45
+import {
46
+ enableCreateEventHandleAPI,
47
+ disableOnScrollBubbling,
48
+} from 'shared/ReactFeatureFlags';
49
50
function extractEvents(
51
dispatchQueue: DispatchQueue,
@@ -165,13 +168,20 @@ function extractEvents(
168
inCapturePhase,
169
);
170
} else {
168
- // TODO: We may also want to re-use the accumulateTargetOnly flag to
169
- // special case bubbling for onScroll/media events at a later point.
170
- // In which case we will want to make this flag boolean and ensure
171
- // we change the targetInst to be of the container instance. Like:
172
- const accumulateTargetOnly = false;
171
+ // Some events don't bubble in the browser.
172
+ // In the past, React has always bubbled them, but this can be surprising.
173
+ // We're going to try aligning closer to the browser behavior by not bubbling
174
+ // them in React either. We'll start by not bubbling onScroll, and then expand.
175
+ let accumulateTargetOnly = false;
176
+ if (disableOnScrollBubbling) {
177
+ accumulateTargetOnly =
178
+ !inCapturePhase &&
179
+ // TODO: ideally, we'd eventually add all events from
180
+ // nonDelegatedEvents list in DOMPluginEventSystem.
181
+ // Then we can remove this special list.
182
+ topLevelType === DOMTopLevelEventTypes.TOP_SCROLL;
183
+ }
184
174
- // We traverse only capture or bubble phase listeners
185
accumulateSinglePhaseListeners(
186
targetInst,
187
dispatchQueue,
packages/shared/ReactFeatureFlags.js
+2
@@ -128,3 +128,5 @@ export const deferRenderPhaseUpdateToNextBatch = true;
128
129
// Replacement for runWithPriority in React internals.
130
export const decoupleUpdatePriorityFromScheduler = false;
131
+
132
+export const disableOnScrollBubbling = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -45,6 +45,7 @@ export const warnAboutSpreadingKeyToJSX = false;
45
export const enableComponentStackLocations = false;
46
export const enableLegacyFBSupport = false;
47
export const enableFilterEmptyStringAttributesDOM = false;
48
+export const disableOnScrollBubbling = false;
49
50
export const enableNewReconciler = false;
51
export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = false;
45
export const enableLegacyFBSupport = false;
46
export const enableFilterEmptyStringAttributesDOM = false;
47
+export const disableOnScrollBubbling = false;
48
49
export const enableNewReconciler = false;
50
export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = true;
45
export const enableLegacyFBSupport = false;
46
export const enableFilterEmptyStringAttributesDOM = false;
47
+export const disableOnScrollBubbling = false;
48
49
export const enableNewReconciler = false;
50
export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = true;
45
export const enableLegacyFBSupport = false;
46
export const enableFilterEmptyStringAttributesDOM = false;
47
+export const disableOnScrollBubbling = false;
48
49
export const enableNewReconciler = false;
50
export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = true;
45
export const enableLegacyFBSupport = false;
46
export const enableFilterEmptyStringAttributesDOM = false;
47
+export const disableOnScrollBubbling = false;
48
49
export const enableNewReconciler = false;
50
export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
44
export const enableComponentStackLocations = true;
45
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
46
export const enableFilterEmptyStringAttributesDOM = false;
47
+export const disableOnScrollBubbling = false;
48
49
export const enableNewReconciler = false;
50
export const deferRenderPhaseUpdateToNextBatch = true;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -18,6 +18,7 @@ export const disableInputAttributeSyncing = __VARIANT__;
18
export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
19
export const enableLegacyFBSupport = __VARIANT__;
20
export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
21
+export const disableOnScrollBubbling = __VARIANT__;
22
23
// Enable this flag to help with concurrent mode debugging.
24
// It logs information to the console about React scheduling, rendering, and commit phases.
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -27,6 +27,7 @@ export const {
27
decoupleUpdatePriorityFromScheduler,
28
enableDebugTracing,
29
enableSchedulingProfilerComponentStacks,
30
+ disableOnScrollBubbling,
31
} = dynamicFeatureFlags;
32
33
// On WWW, __EXPERIMENTAL__ is used for a new modern build.