-
-
Notifications
You must be signed in to change notification settings - Fork 607
fix: Allow undefined ref for SearchBarCommands in SearchBarProps #2724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
Test passed successfully. |
|
hey! thanks for the PR. I took a quick glance I'm not convinced of backward compat of your solution with react 18 yet. |
Ehy no problem. Do you want me to do it using null instead of undefined? I usually use undefined with imperative ref as I prefer it, but yes, maybe react 18 doesn't support undefined, but should definetely support null. If you want I can do it, actually all react 19 users will have this issue, and as far as I know the only way to fix it is provide a nullable (initial) value to the ref. |
| * * `toggleCancelButton` - depending on passed boolean value, hides or shows cancel button (iOS only) | ||
| */ | ||
| ref?: React.RefObject<SearchBarCommands>; | ||
| ref?: React.RefObject<SearchBarCommands | undefined>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should really use React.Ref<SearchBarCommands>, which takes care of all the different shapes of react ref that could be passed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this or mine are both good,the actually used for me is wrong!
This is not breaking as it allows for wider gamut of types to be accepted. Ideally we should allow for `React.Ref` there, as suggested here: #2724 (comment), but it'd require some more internal changes.
This is not breaking as it allows for wider gamut of types to be accepted. Ideally we should allow for `React.Ref` there, as suggested here: #2724 (comment), but it'd require some more internal changes. Co-authored-by: Simone Gauli <[email protected]>
|
This will be done here: #3509 Thanks! |
Description
This change updates the type definition for SearchBarProps to accept a ref that can be undefined. Previously, the ref was forced to be non-nullable, which required developers to use workarounds (like non-null assertions) even though React handles null/undefined refs gracefully. With React 19, the framework ensures that refs are managed correctly regardless of their empty initial state.
By allowing n undefined ref, we align the type definitions with React's natural behavior and improve the developer experience by eliminating unnecessary TypeScript errors. This change has no runtime impact, as the component lifecycle guarantees that the ref will be properly set before it is used.
Example that trigger the Typescript Error below:
Type 'RefObject<SearchBarCommands | undefined>' is not assignable to type 'RefObject'.
Type 'SearchBarCommands | undefined' is not assignable to type 'SearchBarCommands'.
Type 'undefined' is not assignable to type 'SearchBarCommands'.ts(2322)