From 64ddef44c69a18038b1683e04c4558a72af1b91a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Aug 2020 20:54:54 +0100 Subject: [PATCH] Revert "Remove onScroll bubbling flag (#19535)" (#19655) This reverts commit e9721e14e4b8776c107afa3cdd7c6d664fe20c24. --- .../__tests__/ReactDOMEventListener-test.js | 23 ++++++++++++++----- .../ReactDOMEventPropagation-test.js | 6 +++++ .../src/events/plugins/SimpleEventPlugin.js | 21 ++++++++++------- packages/shared/ReactFeatureFlags.js | 1 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 12 files changed, 45 insertions(+), 14 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index cb28d2f252265..5cb207409da73 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -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); } diff --git a/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js b/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js index 06fb657227842..0ef1a8b950d37 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js @@ -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', diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index fa49b84e647a9..dfc345b1d2566 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -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, @@ -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, diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 81975c25896ce..2129245913c19 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -135,3 +135,4 @@ export const enableDiscreteEventFlushingChange = false; // https://github.com/facebook/react/pull/19654 export const enablePassiveEventIntervention = true; +export const disableOnScrollBubbling = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 49f542446f36f..078d1e6b0f43f 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index a050c69e1e1a3..89512478b4515 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index f2a1adc1b4d70..280fc35872905 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 3ede445fa3ddc..aeb589d1b837c 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index acfafaadb255b..1ba4439b28e97 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index ccda44cf343d3..b1b37b557f482 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -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; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 1397d33d53bd9..f58f9f8aae2e6 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -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. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 74d4105ea41ad..d9c237b048a6a 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -28,6 +28,7 @@ export const { enableDebugTracing, skipUnmountedBoundaries, enablePassiveEventIntervention, + disableOnScrollBubbling, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.