fix: add missing getNativeScrollRef type for ScrollView#52203
fix: add missing getNativeScrollRef type for ScrollView#52203zbauman3 wants to merge 1 commit intofacebook:mainfrom
getNativeScrollRef type for ScrollView#52203Conversation
|
Hi @zbauman3! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
getNativeScrollRef type for ScrollView
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
@rshest or @cortinico, it looks like you two have done some previous reviewing on this file. I would love a review/feedback if either of you have time. TYIA! 🙏 |
|
@rshest 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 @zbauman3 in 4b91b63 When will my fix make it into a release? | How to file a pick request? |
Summary: Pull Request resolved: facebook#56718 Update the return type of `getNativeScrollRef` on `ScrollView`, `FlatList`, `SectionList` to be `PublicScrollViewInstance` (previously, a mix of `HostInstance` and `React.ElementRef<>` types which did not include the imperative methods of `ScrollView`). - This type extends `HostInstance & ScrollViewImperativeMethods`, and is what the `ScrollView` ref chain already returns at runtime. - The previous types were either too broad (`HostInstance`), wrong (union with `View`), or required `$FlowFixMe` suppressions. Also removes `ScrollViewNativeComponent` from the public API surface, since it is an internal implementation detail not intended for external use (equivalent props are on the pre-existing `ScrollViewBaseProps` type). Related to: - facebook#52203 - facebook#54735 Changelog: [General][Fixed] - **Strict TypeScript API**: Update `getNativeScrollRef` return type across ScrollView, FlatList, and SectionList Differential Revision: D104223704
Summary:
The Problem
When trying to measure the location of a
Viewwithin aScrollView(ie. for scrolling to the view), the current recommended method is to usemeasureLayouton the nested view to determine its location inside the containing scroll view:This is valid in the Typescript types layer. However, the only two methods on
ScrollViewto use in this scenario that are available in the type definitions aregetScrollableNodeandgetInnerViewNode– both of these methods return anumber. The issue is that anumbernot a valid value to use withmeasureLayoutbecause its source returns early for that type.(Note, you can also use
findNodeHandlewith the scroll view ref, but this also returns anumber.)The Solution
The long-term solution would be to update the types for both
measureLayoutandScrollView. However, that would constitute a breaking change and require some fairly expansive updates. Instead, I am proposing an additive solution.ScrollViewhas a public method calledgetNativeScrollRefwhich returns the underlyingHostInstance. This method correctly works in the runtime layer, but is not supported in the types layer. This PR exposes the public method in the type definition so that we can properly access the underlying instance without using@ts-ignore.Changelog:[GENERAL] [FIXED] - Expose
ScrollView.getNativeScrollRefon the type definition to allow accessing the underlyingHostInstance.Test Plan:
None needed. This is only a type update exposing existing functionality.