[Autocomplete] Warn when mixing controlled/uncontrolled inputValue states #20403
Conversation
useAutocomplete hook assumes, that string value is passed in inputValue property. If an undefined value is passed for whatever case, the client will crash due to length checks against the undefined value
Details of bundle changes.Comparing: 861498c...7242344 Details of page changes
|
|
maybe is better to not crash and throw a big warning? |
|
Added a warning to be logged when inputValue is defaulted to empty string, would that do? |
oliviertassinari
added
the
component: autocomplete
oliviertassinari
changed the title
|
@vileppanen Thanks for starting an effort in this direction. Do you have a reproduction of the issue you are facing? In the meantime, I have pushed the pull request in the direction proposed in #19423. We have seen a non-neglieable number of developers that mix the purpose of the value and input value state (your pull request issue description seems to fall into the same trap, but not sure hence why I have asked for a live reproduction). |
I was almost sure, I had the reproduction ready here |
|
@vileppanen I meant a reproduction where the error originates from within the Autocomplete component. |
Ah yes, it seems to crash in this line https://github.com/mui-org/material-ui/blob/861498c05fc27d794b10803928b8164709772409/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js#L864 (although I'm not sure if that's the only place for causing troubles, as the inputValue is been referred to in other places too) |
|
@vileppanen Ok, thanks, it helps a bit. So, the component is originally controlled, then the prop switches to undefined. So it crashes as expected. The pull request introduces a warning to help developers understand the resolution. |
|
I have the same issue I am using |
Don't recall much of this case, but as I remember, there wasn't an actual "resolution" per se involved, other than enhancing the documentation on how Do you have some repro of your case to look at? |
In freeSolo mode, useAutocomplete hook assumes, that a string value is passed in inputValue property. If an undefined value is passed for whatever reason, the client will crash due to length checks against the undefined value.
Thought that more desirable approach would be to just set the input value as empty string, instead of crashing. In this case, the blame is inherently in the client which is rendering the Autocomplete and misunderstanding the signature of the
onChangefunction. However, it also seems fragile that theuseAutocompletehook assumes the rendering components to pass in "non-null/defined" values.Steps to reproduce:
freeSoloprop to AutocompleteinputValueprop to Autocomplete with value handled in parent componentonChangeprop to Autocomplete with function that will change theinputValueto be undefinedHere's sandbox for the previous example:
https://codesandbox.io/s/autocompleteselectnpe-5mc47
Also, seems that this issue is loosely related to the same thing: for not being prepared to undefined values?
#20152
Closes #19423
Edit by @oliviertassinari