fix(types): expose Modal native ref prop#56889
Conversation
|
@ya-nsh thanks for the contribution, but why is it needed? This increases our API surface, so unless there is a concrete reason why we need it, we would rather not have a new piece of API that we need to maintain. Can you update the summary with the reason why we need the ref prop? |
|
Thanks, that makes sense. I updated the summary with the reason now. The short version is that |
|
Friendly review ping: this PR is ready for maintainer review when you get a chance. Known status: 11 check(s) currently failing; I can follow up if review points to needed changes. Happy to adjust quickly if you want a different shape or narrower scope. |
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D105968862. |
| /** | ||
| * A ref to the native Modal component. | ||
| */ | ||
| modalRef?: React.Ref<PublicModalInstance> | undefined; |
There was a problem hiding this comment.
Can we align with the (few) other uses in our types currently, to use React.RefObject (example).
| modalRef?: React.Ref<PublicModalInstance> | undefined; | |
| modalRef?: React.RefObject<Modal | null> | undefined; |
This then means the usage (in __typetests__/) can be simplified as so.
- // Non-obvious use of HostInstance in lieu of PublicModalInstance not being exported
- const modalRef = React.useRef<HostInstance>(null);
+ // Resolvable via component type under TypeScript
+ const modalRef = React.useRef<Modal>(null);Disclaimer: I'm actually working on a bigger item around ref types for the new Strict TypeScript API in #56673. I'm not 100% certain this type change will endure in the final state, but for now it's aligned with what we have / how other usages are possible today.
Summary
modalRefto the hand-writtenModalTypeScript declarationsPublicModalInstancefromModal.d.tsto match the generated API snapshotHostInstanceref throughmodalRefModalalready acceptsmodalRefat runtime and the generated API snapshot already includes it. The hand-written declarations were the odd one out, so TypeScript users could not pass the supported ref prop without a local cast.Changelog:
[General] [Fixed] - Expose Modal native ref prop in TypeScript declarations
Test Plan
npx prettier --check packages/react-native/Libraries/Modal/Modal.d.ts packages/react-native/types/__typetests__/index.tsxnpm run test-typescript -- --pretty falsenpm run build-typesnpm run test-generated-typescript -- --pretty false