Skip to content

Commit

Permalink
Ship "disableScrollEventThrottleRequirement" (#41676)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #41676

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

fbshipit-source-id: 193019de208f3088519e6f6333dbec4e6b45a1eb
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Nov 29, 2023
1 parent 44109dc commit 56b57e2
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1148,21 +1148,6 @@ class ScrollView extends React.Component<Props, State> {
}

_handleScroll = (e: ScrollEvent) => {
if (__DEV__) {
if (
this.props.onScroll &&
this.props.scrollEventThrottle == null &&
Platform.OS === 'ios'
) {
console.log(
'You specified `onScroll` on a <ScrollView> 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);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@ - (instancetype)initWithFrame:(CGRect)frame

[self.scrollViewDelegateSplitter addDelegate:self];

if (CoreFeatures::disableScrollEventThrottleRequirement) {
_scrollEventThrottle = 0;
} else {
_scrollEventThrottle = INFINITY;
}
_scrollEventThrottle = 0;
}

return self;
Expand Down
4 changes: 0 additions & 4 deletions packages/react-native/React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions packages/react-native/ReactCommon/react/utils/CoreFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ class CoreFeatures {
// Clean yoga node when <Text /> 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;
Expand Down

0 comments on commit 56b57e2

Please sign in to comment.