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

Revert "Remove onScroll bubbling flag (#19535)" #19655

Merged
merged 1 commit into from
Aug 19, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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