Skip to content

Commit

Permalink
Revert "Remove onScroll bubbling flag (#19535)" (#19655)
Browse files Browse the repository at this point in the history
This reverts commit e9721e1.
  • Loading branch information
gaearon authored Aug 19, 2020
1 parent dd651df commit 64ddef4
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 14 deletions.
23 changes: 17 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -712,12 +712,23 @@ describe('ReactDOMEventListener', () => {
bubbles: false,
}),
);
expect(log).toEqual([
['capture', 'grand'],
['capture', 'parent'],
['capture', 'child'],
['bubble', 'child'],
]);
if (gate(flags => flags.disableOnScrollBubbling)) {
expect(log).toEqual([
['capture', 'grand'],
['capture', 'parent'],
['capture', 'child'],
['bubble', 'child'],
]);
} else {
expect(log).toEqual([
['capture', 'grand'],
['capture', 'parent'],
['capture', 'child'],
['bubble', 'child'],
['bubble', 'parent'],
['bubble', 'grand'],
]);
}
} finally {
document.body.removeChild(container);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,12 @@ describe('ReactDOMEventListener', () => {
});

describe('non-bubbling events that do not bubble in React', () => {
// This test will fail outside of the no-bubbling flag
// because its bubbling emulation is currently broken.
// In particular, if the target itself doesn't have
// a handler, it will not emulate bubbling correctly.
// Instead of fixing this, we'll just turn this flag on.
// @gate disableOnScrollBubbling
it('onScroll', () => {
testNonBubblingEvent({
type: 'div',
Expand Down
21 changes: 13 additions & 8 deletions packages/react-dom/src/events/plugins/SimpleEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../EventSystemFlags';
import getEventCharCode from '../getEventCharCode';
import {IS_CAPTURE_PHASE} from '../EventSystemFlags';

import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';
import {
enableCreateEventHandleAPI,
disableOnScrollBubbling,
} from 'shared/ReactFeatureFlags';

function extractEvents(
dispatchQueue: DispatchQueue,
Expand Down Expand Up @@ -182,13 +185,15 @@ function extractEvents(
// In the past, React has always bubbled them, but this can be surprising.
// We're going to try aligning closer to the browser behavior by not bubbling
// them in React either. We'll start by not bubbling onScroll, and then expand.
const accumulateTargetOnly =
!inCapturePhase &&
// TODO: ideally, we'd eventually add all events from
// nonDelegatedEvents list in DOMPluginEventSystem.
// Then we can remove this special list.
// This is a breaking change that can wait until React 18.
domEventName === 'scroll';
let accumulateTargetOnly = false;
if (disableOnScrollBubbling) {
accumulateTargetOnly =
!inCapturePhase &&
// TODO: ideally, we'd eventually add all events from
// nonDelegatedEvents list in DOMPluginEventSystem.
// Then we can remove this special list.
domEventName === 'scroll';
}

accumulateSinglePhaseListeners(
targetInst,
Expand Down
1 change: 1 addition & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,4 @@ export const enableDiscreteEventFlushingChange = false;

// https://github.com/facebook/react/pull/19654
export const enablePassiveEventIntervention = true;
export const disableOnScrollBubbling = true;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const enableComponentStackLocations = false;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
export const skipUnmountedBoundaries = false;
export const disableOnScrollBubbling = true;

export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const enableComponentStackLocations = false;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
export const skipUnmountedBoundaries = false;
export const disableOnScrollBubbling = true;

export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
export const skipUnmountedBoundaries = false;
export const disableOnScrollBubbling = true;

export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
export const skipUnmountedBoundaries = false;
export const disableOnScrollBubbling = true;

export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false;
export const skipUnmountedBoundaries = false;
export const disableOnScrollBubbling = true;

export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const enableComponentStackLocations = true;
export const enableLegacyFBSupport = !__EXPERIMENTAL__;
export const enableFilterEmptyStringAttributesDOM = false;
export const skipUnmountedBoundaries = __EXPERIMENTAL__;
export const disableOnScrollBubbling = true;

export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const enableLegacyFBSupport = __VARIANT__;
export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
export const skipUnmountedBoundaries = __VARIANT__;
export const enablePassiveEventIntervention = __VARIANT__;
export const disableOnScrollBubbling = __VARIANT__;

// Enable this flag to help with concurrent mode debugging.
// It logs information to the console about React scheduling, rendering, and commit phases.
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const {
enableDebugTracing,
skipUnmountedBoundaries,
enablePassiveEventIntervention,
disableOnScrollBubbling,
} = dynamicFeatureFlags;

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

0 comments on commit 64ddef4

Please sign in to comment.