Add type-test for View imperative handle methods#57465
Conversation
|
Hi @sbalagan22! 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! |
b4ac37d to
f02da11
Compare
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
| @@ -0,0 +1,59 @@ | |||
| /** | |||
There was a problem hiding this comment.
nit: Can we rename this file component-refs.tsx?
There was a problem hiding this comment.
Done, renamed to component-refs.tsx.
There was a problem hiding this comment.
nit: Can we rename this file
component-refs.tsx?
Thanks again for taking the time to review and merge this, it was a great first contribution to React Native for me. I'd really like to keep contributing here, so if there are areas that could use help or issues you'd point a newer contributor toward, I'd be glad to pick them up. Would love to stay in touch . https://www.linkedin.com/in/sukhmanbalagan/
A ref to a host component resolves to its imperative handle (ReactNativeElement under the strict public types), which is where the native measurement and mutation methods live. The View component type itself is not that handle, so `React.ComponentRef<typeof View>` is the type that resolves to it under both the legacy and the strict public types. The existing type-tests exercise measure, measureInWindow, and setNativeProps but never measureLayout, so the handle contract had a coverage gap. Add a focused type-test that pins measure, measureInWindow, measureLayout, and setNativeProps on `React.ComponentRef<typeof View>`, checked against both the legacy (types/) and generated strict (types_generated/) definitions.
f02da11 to
886ab81
Compare
Summary:
Calling imperative handle methods on a
Viewref works throughReact.ComponentRef<typeof View>, but the public type-tests only coveredmeasure,measureInWindow, andsetNativeProps, leavingmeasureLayoutwith no type-level regression guard. This adds a focused type-test that pins the full handle contract against both the legacy and the generated strict public types.Root cause of the gap: a ref to a host component resolves to its imperative handle (
ReactNativeElementunder the strict types), not to theViewcomponent type. The component type is a function and does not carry the native methods, so the type that must be exercised isReact.ComponentRef<typeof View>. Without a test assertingmeasureLayouton that type, a change to the handle surface could drop the method from the public types without failing CI.Fix: add
packages/react-native/__typetests__/view-ref.tsxassertingmeasure,measureInWindow,measureLayout, andsetNativePropsonReact.ComponentRef<typeof View>. The file lives in the shared__typetests__directory, so it is type-checked against bothtypes/(legacy) andtypes_generated/(strict, viareact-native-strict-api).On
ViewInstance(per @huntie's note on #54104)Checked how the new
ViewInstance/ per-component*Instancetypes affect this test. Findings from the repo:ViewInstanceis exported today onmain(packages/react-native/Libraries/Components/View/View.js:export type ViewInstance = HostInstance, re-exported fromindex.js.flow) and is present in0.87-stablebut not in0.86-stable, so it ships in 0.87 as noted.types/still modelViewasclass View extends ViewBaseand do not exportViewInstance.ViewInstanceresolves toHostInstance/ReactNativeElement, the same handleReact.ComponentRef<typeof View>resolves to, so it covers all four methods this test pins.React.ComponentRef<typeof View>is not superseded for this purpose and is still used in__typetests__/index.tsx.Because this test is checked against both the legacy and strict variants, it stays on
React.ComponentRef<typeof View>: that is the spelling valid under both, whereasViewInstancewould not resolve under the legacy variant. Once the strict API is the default, a strict-only follow-up could assert the same contract throughuseRef<ViewInstance>directly.Changelog:
[INTERNAL] [ADDED] - Add type-test for View imperative handle methods
Test Plan:
Both type-check suites pass with the new file, and the file lints clean.
Confirmed the test guards the contract: adding a call to a non-existent method on the ref (
view.nonExistentMethod123()) fails both suites withTS2339: Property 'nonExistentMethod123' does not exist on type 'ReactNativeElement'(strict) and... on type 'View'(legacy). Removing that line restores a clean pass.This does not change the behavior reported in #54104 that
useRef<View>().measure()errors under the strict types; per @huntie that is addressed for app code in 0.87 viauseRef<ViewInstance>. This PR guards the existing supported pattern and closes themeasureLayoutcoverage gap.Refs #54104