From 532b4730aa953c90a53966e4d95749e2b00d9af3 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 28 Nov 2023 18:54:33 -0800 Subject: [PATCH] Ship "disableScrollEventThrottleRequirement" (#41676) Summary: This has been used in a significant amount of production for about 2 months, with no consistently statistically significant metric impact. Let's ship it. Note that we would not want to keep this change in a holdout if we remove the warning, since new usages could be added that relied on the behavior not in the holdout. It didn't seem worth the churn to make the same change to Paper, which leaves a question on how to handle the JS-side warning. Instead of jimmying in impl detection, I thought it might be more sane to remove the warning, though that also has a potential hit to Paper DevX. Changelog: [iOS][Changed] - `scrollEventThrottle` no longer needs to be set for continuous scroll events when using the new architecture. Reviewed By: javache Differential Revision: D51608970 --- .../Libraries/Components/ScrollView/ScrollView.js | 15 --------------- .../ScrollView/RCTScrollViewComponentView.mm | 6 +----- .../React/Fabric/RCTSurfacePresenter.mm | 4 ---- .../ReactCommon/react/utils/CoreFeatures.cpp | 1 - .../ReactCommon/react/utils/CoreFeatures.h | 4 ---- 5 files changed, 1 insertion(+), 29 deletions(-) diff --git a/packages/react-native/Libraries/Components/ScrollView/ScrollView.js b/packages/react-native/Libraries/Components/ScrollView/ScrollView.js index 2e2264afedbd2b..30ec946848cc63 100644 --- a/packages/react-native/Libraries/Components/ScrollView/ScrollView.js +++ b/packages/react-native/Libraries/Components/ScrollView/ScrollView.js @@ -1148,21 +1148,6 @@ class ScrollView extends React.Component { } _handleScroll = (e: ScrollEvent) => { - if (__DEV__) { - if ( - this.props.onScroll && - this.props.scrollEventThrottle == null && - Platform.OS === 'ios' - ) { - console.log( - 'You specified `onScroll` on a but not ' + - '`scrollEventThrottle`. You will only receive one event. ' + - 'Using `16` you get all the events but be aware that it may ' + - "cause frame drops, use a bigger number if you don't need as " + - 'much precision.', - ); - } - } this._observedScrollSinceBecomingResponder = true; this.props.onScroll && this.props.onScroll(e); }; diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm index 8272db8c85082c..41e5ebc3cf1a35 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -133,11 +133,7 @@ - (instancetype)initWithFrame:(CGRect)frame [self.scrollViewDelegateSplitter addDelegate:self]; - if (CoreFeatures::disableScrollEventThrottleRequirement) { - _scrollEventThrottle = 0; - } else { - _scrollEventThrottle = INFINITY; - } + _scrollEventThrottle = 0; } return self; diff --git a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm index df069c0865553b..9c2c7aba96a6d5 100644 --- a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm +++ b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm @@ -276,10 +276,6 @@ - (RCTScheduler *)_createScheduler CoreFeatures::enableMountHooks = true; } - if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:disable_scroll_event_throttle_requirement")) { - CoreFeatures::disableScrollEventThrottleRequirement = true; - } - if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_default_async_batched_priority")) { CoreFeatures::enableDefaultAsyncBatchedPriority = true; } diff --git a/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp b/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp index 6bdcc5d651e05e..c967febaac7a21 100644 --- a/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp +++ b/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp @@ -17,7 +17,6 @@ bool CoreFeatures::enableGranularScrollViewStateUpdatesIOS = false; bool CoreFeatures::enableMountHooks = false; bool CoreFeatures::doNotSwapLeftAndRightOnAndroidInLTR = false; bool CoreFeatures::enableCleanParagraphYogaNode = false; -bool CoreFeatures::disableScrollEventThrottleRequirement = false; bool CoreFeatures::enableGranularShadowTreeStateReconciliation = false; bool CoreFeatures::enableDefaultAsyncBatchedPriority = false; bool CoreFeatures::enableClonelessStateProgression = false; diff --git a/packages/react-native/ReactCommon/react/utils/CoreFeatures.h b/packages/react-native/ReactCommon/react/utils/CoreFeatures.h index 2a377625791e66..3ad24c5e02e4d8 100644 --- a/packages/react-native/ReactCommon/react/utils/CoreFeatures.h +++ b/packages/react-native/ReactCommon/react/utils/CoreFeatures.h @@ -47,10 +47,6 @@ class CoreFeatures { // Clean yoga node when does not change. static bool enableCleanParagraphYogaNode; - // Fire `onScroll` events continuously on iOS without a `scrollEventThrottle` - // props, and provide continuous `onScroll` upates like other platforms. - static bool disableScrollEventThrottleRequirement; - // When enabled, the renderer would only fail commits when they propagate // state and the last commit that updated state changed before committing. static bool enableGranularShadowTreeStateReconciliation;