react-native
react-native copied to clipboard
[Native part] Add workaround for android API 33 ANR when inverting ScrollView
Summary:
This PR is a result of this PR, which got merged but then reverted:
- https://github.com/facebook/react-native/pull/37913
We are trying to implement a workaround for #35350, so react-native users on android API 33+ can use <FlatList inverted={true} /> without running into ANRs.
This is the native part, where we add a new internal prop named isInvertedVirtualizedList, which can in a follow up change be used to achieve the final fix as proposed in https://github.com/facebook/react-native/pull/37913
However as @NickGerleman pointed out, its important that we first ship the native change.
Changelog:
[ANDROID] [ADDED] - Native part of fixing ANR when having an inverted FlatList on android API 33+
Test Plan:
- Check the RN tester app and see that scrollview is still working as expected
- Add the
isInvertedVirtualizedListprop as test to a scrollview and see how the scrollbar will change position.
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 9,041,691 | -5,813 |
| android | hermes | armeabi-v7a | 8,292,728 | -4,195 |
| android | hermes | x86 | 9,558,910 | -4,884 |
| android | hermes | x86_64 | 9,400,740 | -5,212 |
| android | jsc | arm64-v8a | 9,601,315 | -4,905 |
| android | jsc | armeabi-v7a | 8,729,512 | -3,422 |
| android | jsc | x86 | 9,689,270 | -3,992 |
| android | jsc | x86_64 | 9,935,285 | -4,260 |
Base commit: 971bb81d131089e3c4201a19f3fa69b24dc72cb0 Branch: main
I think we need this in two more places:
- The type in
ScrollViewNativeComponent? - The ViewConfig (I think also in
ScrollViewNativeComponent).invertedwas already there before for reasons I don't know (it was not passed as a prop before). This runtime structure tells React Native to pass the prop from JS to native.
@NickGerleman thank you for having a look, I updated the files!
[Interestingly this change was already working without these changes (running in fabric on new arch) 👀 ]
Could we update ScrollViewNativeProps here? https://github.com/facebook/react-native/blob/e1fd4a8fcd3c7b06d37dda6890137e344b416aec/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponentType.js#L19
@NickGerleman Updated!
@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Personally, I prefer inverted to internalAndroidApplyInvertedFix. The main reason for this is that we need to pass inverted to the native ScrollView component for react-native-windows and react-native-macos anyway to implement keyboard / mouse wheel friendly inversion.
If @NickGerleman finds inverted to be too general / likely to lead to confusion, then removing Android from the prop name and instead using something like isInvertedVirtualizedList would be prefered.
In fact, this isn't the first thing we've wanted to pass context that the ScrollView is responsible for rendering a VirtualizedList / FlatList before. In react-native-windows, we wanted to build in functionality to default to arrow-key based keyboard navigation, but only when rendered in the context of a FlatList.
@rozele I don't have a opinion on that and go with what the you the core maintainers are saying 😅
For now I renamed it to isInvertedVirtualizedList, happy to change that any time to something else if needed!
@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
This pull request was successfully merged by @hannojg in 6d206a3f54725f7f53692222293a9d1e58b11ca4.
When will my fix make it into a release? | Upcoming Releases