[useAutocomplete] Improve isOptionEqualToValue value argument type#47801
[useAutocomplete] Improve isOptionEqualToValue value argument type#47801silviuaavram wants to merge 3 commits intomui:masterfrom
Conversation
Netlify deploy previewBundle size report
|
There was a problem hiding this comment.
Pull request overview
This PR improves the TypeScript type definitions for the isOptionEqualToValue and getOptionLabel props in the Autocomplete component to correctly reflect that in freeSolo mode, the value parameter can be either the option type or a string. This addresses a type safety issue where developers using freeSolo mode were not getting accurate type information.
Changes:
- Introduced a new
AutocompleteValueOrFreeSoloValueMappingtype helper that resolves toValue | stringwhenFreeSoloistrue, otherwise justValue - Updated type definitions and documentation for
getOptionLabelandisOptionEqualToValueto use this new type helper - Added demo examples showing proper usage of
isOptionEqualToValuewith freeSolo mode
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mui-material/src/useAutocomplete/useAutocomplete.d.ts | Added new type helper and updated type definitions for getOptionLabel and isOptionEqualToValue |
| packages/mui-material/src/Autocomplete/Autocomplete.js | Updated JSDoc comments to reflect that parameters can be Value or string |
| packages/mui-joy/src/Autocomplete/AutocompleteProps.ts | Updated import to use new type helper and applied it to getOptionLabel |
| packages/mui-joy/src/Autocomplete/Autocomplete.tsx | Updated JSDoc comments to reflect that parameters can be Value or string |
| docs/pages/material-ui/api/autocomplete.json | Updated API documentation signatures to show Value or string types |
| docs/pages/joy-ui/api/autocomplete.json | Updated API documentation signatures to show Value or string types |
| docs/data/material/components/autocomplete/FreeSolo.tsx | Added example demonstrating isOptionEqualToValue with freeSolo mode |
| docs/data/material/components/autocomplete/FreeSolo.js | Added example demonstrating isOptionEqualToValue with freeSolo mode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| AutocompleteFreeSoloValueMapping, | ||
| AutocompleteInputChangeReason, | ||
| AutocompleteValue, | ||
| AutocompleteValueOrFreeSoloValueMapping, |
There was a problem hiding this comment.
This import references AutocompleteValueOrFreeSoloValueMapping from the external @mui/base package. Please verify that this type has been added to @mui/base (version 7.0.0-beta.4 or the version being used) before merging this PR. If this type doesn't exist in the current version of @mui/base, this will cause a TypeScript compilation error.
siriwatknp
left a comment
There was a problem hiding this comment.
same here, need to add to upgrading guide
d5ffd74 to
0a76519
Compare
108071c to
5ef4df5
Compare
siriwatknp
left a comment
There was a problem hiding this comment.
When it's related to types, I recommend adding .spec.tsx to confirm the behavior.
In this case, let's add a fail case (missing typeof check) and a success case (similar to the demo but can be shorter).
Take a look at packages/mui-material/src/Autocomplete/Autocomplete.spec.tsx
5ef4df5 to
62319d5
Compare
|
|
||
| return option === value; | ||
| }} | ||
| />; |
There was a problem hiding this comment.
| />; | |
| />; | |
| <Autocomplete | |
| id="free-solo-demo3" | |
| freeSolo | |
| options={top100Films} // array of object | |
| renderInput={() => null} | |
| // @ts-expect-error option could be a string | |
| getOptionLabel={(option) => option.title} | |
| isOptionEqualToValue={(option, value) => { | |
| // @ts-expect-error option could be a string | |
| return option.title === value?.title; | |
| }} | |
| /> |
If my understanding is correct, this should be added to ensure typesafety right?
siriwatknp
left a comment
There was a problem hiding this comment.
👍 Please check my last comment related to the spec.
| if (typeof value === 'string') { | ||
| return option.title === value; | ||
| } | ||
| return option.title === value?.title; |
There was a problem hiding this comment.
Is this optional or no?
| return option.title === value?.title; | |
| return option.title === value.title; |
It's this PR #37291 from @newsiberian but the files have changed since then so created a new PR with the changes on the updated file structure. Same changes, but:
Fixes #38322