-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
[useAutocomplete] Improve isOptionEqualToValue value argument type
#37291
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
base: v5.x
Are you sure you want to change the base?
[useAutocomplete] Improve isOptionEqualToValue value argument type
#37291
Conversation
… could be a string for `freeSolo`
Netlify deploy previewhttps://deploy-preview-37291--material-ui.netlify.app/ Bundle size report |
|
Is there an issue you ran into because of this? IIRC, the |
|
@michaldudak , yes, this is my issue (with |
ZeeshanTamboli
left a comment
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.
@newsiberian Could you please provide a CodeSandbox reproducing the issue? The issue template is a good starting point.
|
There is an issue created independently that describes the problem: #38322 @newsiberian Your solution seems to be correct. Please rebase this PR on the latest master and make sure the checks pass. |
ZeeshanTamboli
left a comment
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.
The changes make sense, but it will be a breaking change for apps using Autocomplete with both freeSolo and this prop. TypeScript will throw a compile error now, and users will need to update the value parameter in isOptionEqualToValue using type narrowing. Here's an example:
<Autocomplete
freeSolo
options={options}
getOptionLabel={(option) => (typeof option === 'string' ? option : option.book.name)}
isOptionEqualToValue={(option, value) =>
option.book.id === (typeof value !== 'string' ? value.book.id : value)
}
renderInput={(params) => <TextField {...params} label="book" />}
/>;Yes it throws a runtime error as mentioned in #38322 but they might be using optional chaining to fix the runtime error.
isOptionEqualToValue value argument typeisOptionEqualToValue value argument type
…-option-equal-to-value-free-solo-type-fix # Conflicts: # packages/mui-base/src/useAutocomplete/useAutocomplete.d.ts
|
I agree with @ZeeshanTamboli in that this is a breaking change that should be released in a major version. Given that the solution is ready, we could target this for v6. |
…lue declaration for `isOptionEqualToValue` and `getOptionLabel` to `AutocompleteValueOrFreeSoloValueMapping` and add new demo which covers cases when option can change it type to a string
|
@ZeeshanTamboli , all set. tests were fine |
ZeeshanTamboli
left a comment
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.
@newsiberian Can you fix the CI?
…tocompleteFreeSoloValueMapping<FreeSolo>)` to `(option: AutocompleteValueOrFreeSoloValueMapping<T, FreeSolo>)`
|
Hey again! After discussing with members of the core team, I confirm that we won't release this in v5. The breaking change is to fix a bug, but it's a breaking change nonetheless. I'm really sorry 😓. The good news is that we plan to start working on v6 very soon, so we could have this in early alpha releases, with the stable release planned for Q2 2024. We also plan to have more frequent major releases so we don't have to hold bug fixes that require breaking changes like this for too long in the future. |

I've noticed that
isOptionEqualToValuedoesn't take into account that value could be a string (freeSolomode)Fixes #38322