Material-ui: Console Warn when more than one Input is used in a FormControl

[lemes]

• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Whenever we have something like this:

<FormControl>
  <Input />
  <Input />
</FormControl>

we should console a proper message explaining how to fix (maybe link to docs too?). Possibly, we could also prevent the _Maximum update depth exceeded_ error so that users don't get confuse with several errors in the console.

Plus, the verification can (or should) only exist in development mode.

Motivation 🔦

The _Maximum update depth exceeded_ error message doesn't really help much when you have a long tree of components and you aren't the one who wrote the code. Especially if you aren't aware of the FormControl limitation (like I didn't).

It took me sometime to figure it out. I was mostly debugging other parts of my code but I eventually ran out of options. After that I was hopeless so I started removing parts of the code... and that's when I found it. From here, knowing where to look for, I was able to find this issue #12197.

TL;DR
I recently had to fix an issue caused by having multiple inputs inside a FormControl. Fixing it was easy, finding what was going on definitely wasn't it. I believe that by handling this error and prompting the user with the right directions we could help save a few hours of debugging.

[oliviertassinari]

@lemes I agree, any idea on how we could implement this warning?

[lemes]

@oliviertassinari initially I though we could loop through FormControl children and count the inputs (using isMuiElement), if greater than one, warn/error the problem in the console. However, that means any nested structured would fail, such as:

<FormControl>
  <Input /> { /* hey one input! noted. */ }
  <div> { /* div is not a mui component */ }
    <Input /> { /* React.Children.forEach can't reach here 🤷‍♂️ */ }
  </div>
</FormControl>

Given that limitation, my proposal idea is to register the input within the provider once the input mount and unregister when it unmount. Ref would be enough to hold that flag (and a bonus of no rerender). Whenever a second input tries to register itself, we check the ref flag and warn/error in the console.

// add to FormControl
const registeredInput = useRef(false)
const register = () => {
  if (registeredInput.current) { // whether an input was already registered
    warning(/* */) 
  }

  registeredInput.current = true

  return () => {
    registeredInput.current = false
  }
}

const childContext = {
  // ...
 register
}

// add to input mui components
useEffect(() => {
  const unregister = formControl.register()
  return unregister
}, []) // runs once on mount

Now, I'm not sure is what message to show and whether we should throw or just warn, but if you think that is a valid approach on how to solve then I'll gladly submit an initial PR for further discussion.

[oliviertassinari]

@lemes The registration approach in an effect should work. It's a great idea! I would also wrap it with a process.env.NODE_ENV !== 'production' check. Regarding the message, we could say.

Material-UI: there are multiple InputBase components inside a FromControl. This is not supported. It might cause infinite rendering loops. Only use one InputBase.

Do you want to give a pull request a shot?

[lemes]

Cool! Yes, I'd like to draft a PR 😄

[lemes]

@oliviertassinari how about the Switch component? It ins't build on top of InputBase and multiple Switches inside the same FromControl works (with or without a FormGroup).

Placing a Switch and any InputBase component inside the same FormControl also works (no loop exception). However, the result can be a bit weird because part of the state is shared, for example, when focusing on the Switch component, the input will also render as if it is also focused.

You can check it here: https://stackblitz.com/edit/rfmauf

My question is, should we also warn when using Switch or should we leave as it is, considering that the UI glitching will already hint the problem? If we chose to warn, then it gets complicated as we do support multiple Switches.

[oliviertassinari]

@lemes We actually have a demo for it: https://material-ui.com/components/switches/#switches-with-formgroup, same with the checkbox. I think that it's fine.

[borm]

Hey there, @oliviertassinari @lemes , i need use two InputBase,
one real
second for drop-down

But my both input render inside one FormControl and i can`t change this

So if u did this warning u must think about my case and allow me do something like useFormControl={false} prop for second input