Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
p-himik opened this issue Aug 5, 2020 · 13 comments
Closed
2 tasks done
Labels
bug 馃悰 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@p-himik
Copy link

p-himik commented Aug 5, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

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
@p-himik p-himik added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 5, 2020
@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 5, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 5, 2020

@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 facebook/react#17070, mentioning undefined in the error would help.

@p-himik
Copy link
Author

p-himik commented Aug 5, 2020

(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
Copy link
Member

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

@p-himik
Copy link
Author

p-himik commented Aug 5, 2020

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

This comment has been minimized.

@p-himik

This comment has been minimized.

@MincePie

This comment has been minimized.

@MincePie

This comment has been minimized.

@p-himik

This comment has been minimized.

@MincePie

This comment has been minimized.

@oliviertassinari oliviertassinari self-assigned this Aug 16, 2020
@oliviertassinari
Copy link
Member

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 oliviertassinari added bug 馃悰 Something doesn't work ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Sep 15, 2020
@oliviertassinari
Copy link
Member

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 oliviertassinari removed their assignment Sep 15, 2020
@oliviertassinari oliviertassinari changed the title "A component is changing the controlled value state of Autocomplete to be uncontrolled" error [Autocomplete] "A component is changing the controlled value state to be uncontrolled" error Oct 10, 2020
@oliviertassinari
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

No branches or pull requests

3 participants