Material-ui: [Autocomplete] Warn when mixing uncontrolled and controlled API

[JDansercoer]

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

Current Behavior 😯

When passing in a value of undefined and then changing the value to a correct value, stops the AutoComplete from re-rendering.

Expected Behavior 🤔

At least something to happen. Now you're just stuck trying to figure out what's happening. Either default to an empty array, of have something logged in console to warn about it (e.g. like React does when changing from uncontrolled to controlled).

Steps to Reproduce 🕹

https://codesandbox.io/embed/material-demo-dj0w5

Steps:

1. Check the console. See that the value is undefined.
2. Click the button at the top.
3. See that the value is now an array with two elements.
4. AutoComplete is not showing the new values.
5. If you change the initial useState() value to useState([]) everything works as expected.

Context 🔦

Our values are fetched from an api. So initially the value is not defined. We could set our initial state to an empty array, but this is conflicting, because that way you don't know when the api call has completed (as an empty array could also be returned by the api call).

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | latest |
| React | latest |
| Browser | chrome |
| TypeScript | no |

[oliviertassinari]

@JDansercoer Oh yes, thank you for the report! I had forgotten to add the warning for this one. If you want to contribute, you can go ahead :)

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 1d947d6ff..38635d673 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -164,6 +164,25 @@ export default function useAutocomplete(props) {
   });
   const value = isControlled ? valueProp : valueState;

+  if (process.env.NODE_ENV !== 'production') {
+    // eslint-disable-next-line react-hooks/rules-of-hooks
+    React.useEffect(() => {
+      if (isControlled !== (valueProp != null)) {
+        console.error(
+          [
+            `Material-UI: A component is changing ${
+              isControlled ? 'a ' : 'an un'
+            }controlled useAutocomplete to be ${isControlled ? 'un' : ''}controlled.`,
+            'Elements should not switch from uncontrolled to controlled (or vice versa).',
+            'Decide between using a controlled or uncontrolled useAutocomplete ' +
+              'element for the lifetime of the component.',
+            'More info: https://fb.me/react-controlled-components',
+          ].join('\n'),
+        );
+      }
+    }, [valueProp, isControlled]);
+  }
+
   const [inputValue, setInputValue] = React.useState('');
   const [focused, setFocused] = React.useState(false);

[JDansercoer]

Before starting on a PR: is it enough to just document this with a warning?
Because React also throws warnings when changing from uncontrolled to controlled, but the core functionality still works. However, with the Autocomplete component, the component does not work.

[oliviertassinari]

@JDansercoer We already have 5 occurrences of this exact logic in the codebase. It does warn in both directions.

[oliviertassinari]

but the core functionality still works

I would challenge this affirmation.

[JDansercoer]

I would challenge this affirmation.

What exactly do you mean by this?

[oliviertassinari]

You are right it does work with React + <input>: https://codesandbox.io/s/material-demo-wocx2.

It doesn't work with Material-UI. It's a simplification. Should we change it? At the very least, a new warning would be a great first step (use null instead of undefined). I think that we can consider the non-docs approach later on if we get more user feedback.

[goodwin64]

For those who also faced this issue there is a simple workaround: use null value instead of undefined:

export default function Playground() {
  const defaultProps = {
    options: top100Films,
    getOptionLabel: (option: FilmOptionType) => option.title
  };

  // doesn't work
  // const [value, setValue] = React.useState();

  // works fine
  const [value, setValue] = React.useState(null);

  return (
    <div style={{ width: 300 }}>
       <Autocomplete
        {...defaultProps}
        value={value}
        onChange={(event, newValue) => {
          setValue(newValue);
        }}
        renderInput={params => (
          <TextField {...params} label="controlled" margin="normal" fullWidth />
        )}
      />
      <Autocomplete
        {...defaultProps}
        value={value}
        onChange={(event, newValue) => {
          setValue(newValue);
        }}
        renderInput={params => (
          <TextField {...params} label="controlled2" fullWidth />
        )}
      />
    </div>
  );
}

[oliviertassinari]

Does anyone want to submit a pull request for this problem, based on https://github.com/mui-org/material-ui/issues/18173#issuecomment-549312720?

[divrt-srinath]

I am not using useState, with the below code its not working. What am I doing wrong here?

<Autocomplete
     value={this.state.autoC}
     id="combo-box-demo"
     options={[
            { title: 'red', year: 1994 },
            { title: 'blue', year: 1972 }]}
      getOptionLabel={option => option.title}
      onChange={(evt,value)=>{
            this.setState({
              autoC:value
            })
          }}
       renderInput={params => (
            <TextField {...params} label="Combo box" variant="outlined" fullWidth />
          )}
 />

Edit: I just copied the example from the site, it does not display value

[yuechen]

I actually don't think this warning is appropriate. It can be totally expected behavior to change the variable passed into defaultValue without expecting the value in the Autocomplete to change. Material UI should not specify this for the developer, and should definitely not throw an error for this (as is currently the behavior). Could be more acceptable as a warning.

[oliviertassinari]

Could be more acceptable as a warning.

@yuechen This resonates with https://github.com/mui-org/material-ui/issues/15343#issuecomment-538759792.

[RmStorm]

I was running into this exact same problem, where I fetch the valid values for a state from an API and so I have to instantiate the state in some empty manner. Basically {} and [] and null work but undefined doesn't. This was a little surprising to me and also not obvious from the documentation linked in the warning.

I expected that since I am explicitly setting the value using value={this.state.myValue} and I instantiate myValue as undefined it will be a controlled component. Maybe this peculiarity should actually be in the warning. Since I assumed I had made a controlled component but by using undefined I actually hadn't.

But the warning is used more general then this specific case so that doesn't seem the place to fix it. It might be best if undefined would be considered a valid default value for a controlled component. But honestly, I have just started learning React so I'm way out of my depth. The current warning is definitely better then no warning but when new users run into it now, it's probably still not clear why the component is switching from uncontrolled to controlled. @oliviertassinari thoughts?

[oliviertassinari]

@RmStorm Thank you for taking the time to write in details your experience. I fear it's expected, however, your mention to the link not helping can be leveraged. I have updated https://github.com/mui-org/material-ui/issues/19423#issuecomment-589862301 to have a mention about it. Thanks (also, feel free to work on it if you want to help us improve the library :)).

[Alarees]

When passing in a value of undefined and then changing the value to a correct value, stops the AutoComplete from re-rendering.

thanks @JDansercoer , this save my day
now i change my code to
value={formState.attributes[item] || ""}