Skip to content

Commit 06d104e

Browse files
gaearongaearon
authored
Don't emulate bubbling of the scroll event (facebook#19464)
* Don't emulate bubbling of the scroll event * Put behind a flag
1 parent 217ecf5 commit 06d104e

11 files changed

+132
-16
lines changed

‎packages/react-dom/src/__tests__/ReactDOMEventListener-test.js

Lines changed: 105 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,6 @@ describe('ReactDOMEventListener', () => {
539539
const container = document.createElement('div');
540540
const ref = React.createRef();
541541
const onPlay = jest.fn();
542-
const onScroll = jest.fn();
543542
const onCancel = jest.fn();
544543
const onClose = jest.fn();
545544
const onToggle = jest.fn();
@@ -548,14 +547,12 @@ describe('ReactDOMEventListener', () => {
548547
ReactDOM.render(
549548
<div
550549
onPlay={onPlay}
551-
onScroll={onScroll}
552550
onCancel={onCancel}
553551
onClose={onClose}
554552
onToggle={onToggle}>
555553
<div
556554
ref={ref}
557555
onPlay={onPlay}
558-
onScroll={onScroll}
559556
onCancel={onCancel}
560557
onClose={onClose}
561558
onToggle={onToggle}
@@ -568,11 +565,6 @@ describe('ReactDOMEventListener', () => {
568565
bubbles: false,
569566
}),
570567
);
571-
ref.current.dispatchEvent(
572-
new Event('scroll', {
573-
bubbles: false,
574-
}),
575-
);
576568
ref.current.dispatchEvent(
577569
new Event('cancel', {
578570
bubbles: false,
@@ -591,7 +583,6 @@ describe('ReactDOMEventListener', () => {
591583
// Regression test: ensure we still emulate bubbling with non-bubbling
592584
// media
593585
expect(onPlay).toHaveBeenCalledTimes(2);
594-
expect(onScroll).toHaveBeenCalledTimes(2);
595586
expect(onCancel).toHaveBeenCalledTimes(2);
596587
expect(onClose).toHaveBeenCalledTimes(2);
597588
expect(onToggle).toHaveBeenCalledTimes(2);
@@ -643,4 +634,109 @@ describe('ReactDOMEventListener', () => {
643634
document.body.removeChild(container);
644635
}
645636
});
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+
});
646742
});

‎packages/react-dom/src/events/plugins/SimpleEventPlugin.js

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../EventSystemFlags';
4242
import getEventCharCode from '../getEventCharCode';
4343
import {IS_CAPTURE_PHASE} from '../EventSystemFlags';
4444

45-
import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';
45+
import {
46+
enableCreateEventHandleAPI,
47+
disableOnScrollBubbling,
48+
} from 'shared/ReactFeatureFlags';
4649

4750
function extractEvents(
4851
dispatchQueue: DispatchQueue,
@@ -165,13 +168,20 @@ function extractEvents(
165168
inCapturePhase,
166169
);
167170
} 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+
}
173184

174-
// We traverse only capture or bubble phase listeners
175185
accumulateSinglePhaseListeners(
176186
targetInst,
177187
dispatchQueue,

‎packages/shared/ReactFeatureFlags.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,5 @@ export const deferRenderPhaseUpdateToNextBatch = true;
128128

129129
// Replacement for runWithPriority in React internals.
130130
export const decoupleUpdatePriorityFromScheduler = false;
131+
132+
export const disableOnScrollBubbling = false;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export const warnAboutSpreadingKeyToJSX = false;
4545
export const enableComponentStackLocations = false;
4646
export const enableLegacyFBSupport = false;
4747
export const enableFilterEmptyStringAttributesDOM = false;
48+
export const disableOnScrollBubbling = false;
4849

4950
export const enableNewReconciler = false;
5051
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = false;
4545
export const enableLegacyFBSupport = false;
4646
export const enableFilterEmptyStringAttributesDOM = false;
47+
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;
4950
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = false;
4646
export const enableFilterEmptyStringAttributesDOM = false;
47+
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;
4950
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = false;
4646
export const enableFilterEmptyStringAttributesDOM = false;
47+
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;
4950
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.testing.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = false;
4646
export const enableFilterEmptyStringAttributesDOM = false;
47+
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;
4950
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.testing.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export const warnAboutSpreadingKeyToJSX = false;
4444
export const enableComponentStackLocations = true;
4545
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
4646
export const enableFilterEmptyStringAttributesDOM = false;
47+
export const disableOnScrollBubbling = false;
4748

4849
export const enableNewReconciler = false;
4950
export const deferRenderPhaseUpdateToNextBatch = true;

‎packages/shared/forks/ReactFeatureFlags.www-dynamic.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export const disableInputAttributeSyncing = __VARIANT__;
1818
export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
1919
export const enableLegacyFBSupport = __VARIANT__;
2020
export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
21+
export const disableOnScrollBubbling = __VARIANT__;
2122

2223
// Enable this flag to help with concurrent mode debugging.
2324
// It logs information to the console about React scheduling, rendering, and commit phases.

‎packages/shared/forks/ReactFeatureFlags.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export const {
2727
decoupleUpdatePriorityFromScheduler,
2828
enableDebugTracing,
2929
enableSchedulingProfilerComponentStacks,
30+
disableOnScrollBubbling,
3031
} = dynamicFeatureFlags;
3132

3233
// On WWW, __EXPERIMENTAL__ is used for a new modern build.

0 commit comments

Comments
 (0)