feat: Improved types for react useIonModal
Prerequisites
- [X] I have read the Contributing Guidelines.
- [X] I agree to follow the Code of Conduct.
- [X] I have searched for existing issues that already include this feature request, without success.
Describe the Feature Request
useIonModal should type props parameter.
Describe the Use Case
useIonModal currently allows any for component props. This is dangerous and has caused many crashes and unintended behaviors in my experience.
Currently:
export declare function useIonModal(component: ReactComponentOrElement, componentProps?: any): UseIonModalResult;
Describe Preferred Solution
export declare function useIonModal<P>(component: React.ComponentClass<P> | React.FC<P>, componentProps?: P): UseIonModalResult;
export declare function useIonModal(component: React.ReactElement, componentProps?: any): UseIonModalResult;
Describe Alternatives
Leaving as-is
Related Code
No response
Additional Information
If you would accept this change, I would happily make a PR
FYI I see #27548, this seemed a bit more open-ended so I decided to make a separate issue in smaller scope (fixing the types).
Thanks! I think this would be a good change to make. We'll want to do make this change for both useIonModal and useIonPopover, and T should continue to be any if not set to avoid breaking changes.
@liamdebeasi, in my testing Typescript automatically infers T based on the component you pass it. By making the proposed change I was able to see type errors with incorrect params in my usage without adding any explicit generic declarations to my code :)
in my testing Typescript automatically infers T based on the component you pass it.
I'm aware, I was just clarifying that we shouldn't default it to anything else 😄
This issue has been labeled as help wanted. This label is added to issues that we believe would be good for contributors.
If you'd like to work on this issue, please comment here letting us know that you would like to submit a pull request for it. This helps us to keep track of the pull request and make sure there isn't duplicated effort.
For a guide on how to create a pull request and test this project locally to see your changes, see our contributing documentation.
Thank you!
Is this still relevant?
Yes, and my PR was close to being merged, but @liamdebeasi left Ionic so no progress since
Ah, thank you. I'm new here and digging around for ways to help. So your PR partially completed the feature, but there is some left to do?