search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.96k stars 32.27k forks source link

[Autocomplete] "A component is changing the controlled value state to be uncontrolled" error #22073

Closed p-himik closed 4 years ago

p-himik commented 4 years ago

Current Behavior 😯

If the list of Autocomplete options is changed when some other than first value is highlighted, pressing Enter will result in the error in the title.

Material-UI: A component is changing the controlled value state of Autocomplete to be uncontrolled.
Elements should not switch from uncontrolled to controlled (or vice versa).
Decide between using a controlled or uncontrolled Autocomplete element for the lifetime of the component.
The nature of the state is determined during the first render, it's considered controlled if the value is not `undefined`.
More info: https://fb.me/react-controlled-components 
    in Autocomplete (created by WithStyles(ForwardRef(Autocomplete)))
    in WithStyles(ForwardRef(Autocomplete)) (at demo.js:44)
    in BrokenAutocomplete (at index.js:6)

Expected Behavior 🤔

There should be no error - you should be able to select any option just fine.

Steps to Reproduce 🕹

  1. Go to https://codesandbox.io/s/material-demo-kw16l
  2. Put text cursor in the Autocomplete input field that should read "abcf"
  3. Delete the character "f". Now the pop-up should display all options
  4. Type "f" once again. Now the pop-up should display only the "abcf" option
  5. Hit Enter. The input should disappear completely and the console should have the above error

After some debugging, it seems that the root cause is that the selected item index is not recomputed on options change. Take a look at this dependency list of the relevant effect: https://github.com/mui-org/material-ui/blob/9bd4277ecd660ebe2fd4cb08960f58e98ddb6d43/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js#L457-L464 As you can see, it only cares about whether there are any options at all and not about the number of options.

Context 🔦

I'm trying to implement a simple autocomplete that loads its options via network.

Your Environment 🌎

Tech Version
Material-UI v4.11.0 / v5.0.0-alpha5
Material-UI lab v4.0.0-alpha56 / v5.0.0-alpha5
React v16.3.1
oliviertassinari commented 4 years ago

@p-himik This one is tough, it's an issue with the syncHighlightedIndex logic. When the error happens (returning undefined value in onChange), the highlight is on the 3rd option, but there is only 1 displayed because of the fetch in a useEffect.

I don't know what would be the best solution in this case:

  1. Force options to be memorized, we can include it in the effect logic, I'm not sure about the implications.
  2. Add an API to resync the highlight, similar to resetAfterIndex of https://react-window.now.sh/#/api/VariableSizeList.
  3. We add a new effect to guarantees we stay in the possible boundaries.
  4. Else?

On a side note, this makes me think of https://github.com/facebook/react/pull/17070, mentioning undefined in the error would help.

p-himik commented 4 years ago

(2) seems very counter-intuitive. Autocomplete is already complicated enough so that it's almost impossible to use it correctly without studying the API documentation and the examples.

(1) was my immediate thought, because it just seems logical. And I don't know about the implications as well. But right now it seems that it's worth pursuing it simply because it's logical. (3) is in the same camp, I'd say it's just implementation details as long as they achieve the same goal and don't change the API.

mentioning undefined in the error would help.

Right, my bad - I've added the error message to the OP.

oliviertassinari commented 4 years ago

@p-himik Cool, do you want to keep exploring the scope of the problem? :)

p-himik commented 4 years ago

To be honest, not really - I'm trying to do my best at avoiding going outside of ClojureScript. Especially when it comes to unfamiliar and complicated components with long history.

MincePie commented 4 years ago

Did you figure this out? I'm having a similar problem with both autocomplete and select options. https://stackoverflow.com/users/2860931/mel

p-himik commented 4 years ago

@MincePie As a workaround I just toggle filterSelectedOptions back and forth to trigger the refresh of the currently selected index when options change.

MincePie commented 4 years ago

@p-himik hmmm - my options are static - so potentially need a different solution. Thanks for sharing yours.

MincePie commented 4 years ago

Hi @p-himik - I'm not permitted to raise an issue on this forum.

Please can you share the code that you were able to figure out to use the Autocomplete field.

I'm stuck - trying to use it with static options. https://stackoverflow.com/questions/63312956/formik-material-ui-react-autocomplete-uncontrolled-to-controlled-state

Any insights would be very much appreciated. Thank you.

p-himik commented 4 years ago

@MincePie You're using Formic and React with JavaScript, plus you have static options. I'm using just Material UI and with Reagent and ClojureScript, plus my problem is with dynamic options in particular. My code will not do you any good.

Regarding insights - the first thing that I would try is to get rid of Formic and just use Autocomplete by itself. Because if that works (and it should), then it means that the issue is with Formic and not with Material UI.

MincePie commented 4 years ago

@p-himik - okay, thank you for considering my request for help.

The whole point of this tool is supposed to be that it is built to support formik. When I don't use this tool, I can have an Autocomplete field that works. When I try to build a form with Formik that uses Autcomplete, I'm expecting this tool to support that attempt. I've been unable to find a solution that problem thus far.

I appreciate that your issue was different to mine. I just thought seeing the code might reveal something about how to adapt this tool to actually work.

oliviertassinari commented 4 years ago

The problem is very close to #22170, we are discussing the solutions possible in https://github.com/mui-org/material-ui/issues/22170#issuecomment-691825640

oliviertassinari commented 4 years ago

Since https://github.com/mui-org/material-ui/issues/22170#issuecomment-691901855 we have a proposal that should, in theory, work. It's up for grab.

oliviertassinari commented 4 years ago

The reproduction is now working correctly on the latest version (v5.0.0-alpha.12): https://codesandbox.io/s/material-demo-forked-t5j4w?file=/demo.js. I don't know when the issue was solved.