[TextField] focused={true} and disabled={true} causes an infinite render

[ameem91]

If you render a TextField with the focused and disabled flags both set to true, it causes an infinite render.

• 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 😯

Code:

<TextField id="standard-basic" label="Standard" focused disabled />

Error:

Too many re-renders. React limits the number of renders to prevent an infinite loop.

Expected Behavior 🤔

While it probably doesn't make sense to set both focused and disabled at the same time, I believe that the current error (too many re-renders) is not correct behavior. It's breaks the app and is difficult to debug. Instead, we can default to one of focused or disabled if both are set and add a console warning.

Steps to Reproduce 🕹

https://codesandbox.io/s/material-ui-issue-forked-kd970?file=/src/Demo.js

[eps1lon]

Thanks for the report.

While it probably doesn't make sense to set both focused and disabled at the same time [...]

I definitely agree. We should just remove this (and similar props). State should live in a single place (context, props, actual state). Mixing it, makes it hard to reason about and bloats the implementation needlessly.

I'm adding this to #24450 which already tracks this problem space.

[oliviertassinari]

is difficult to debug

@ameem91 It does look difficult, indeed. I can't find any information in the error that could allow a developer to figure out the origin without going into a long trial & error exploration. I think that we can look into making it easier to spot.

Looking at the source, it seems that the infinite loop comes from a wrong branching logic. I believe it should have been:

diff --git a/packages/material-ui/src/FormControl/FormControl.js b/packages/material-ui/src/FormControl/FormControl.js
index e0da18ccfc..b5575867bd 100644
--- a/packages/material-ui/src/FormControl/FormControl.js
+++ b/packages/material-ui/src/FormControl/FormControl.js
@@ -161,7 +161,7 @@ const FormControl = React.forwardRef(function FormControl(inProps, ref) {
   const [focusedState, setFocused] = React.useState(false);
   const focused = visuallyFocused !== undefined ? visuallyFocused : focusedState;

-  if (disabled && focused) {
+  if (disabled && focusedState) {
     setFocused(false);
   }

So disabled wins over focused and only resets what it's allowed to.

---

On a larger note raised by Sebastian. The focused prop allows setting the state from the outside for components. It's important to support composite components. For instance, a date range picker built with two textboxes. The DOM focus can be inside the popup, not on a textbox, and yet, the popup controls the state of one of the two textboxes: #20276.

We could add a warning when focused and disabled are both set from the outside. I would personally not spend time on it, but why not.

[eps1lon]

The DOM focus can be inside the popup, not on a textbox, and yet, the popup controls the state of one of the two textboxes:

Then the TextField is not focused and should still not allow the focused prop. The DateRangePicker is just under-specified at the moment and the problem space not thoroughly explored. The focused prop is abused right now and not used properly. The DateRangePicker using it, is not an argument that the existence of the API is correct. That is just never an argument.

Edit: Closing since it is still tracked as part of #24450

[oliviertassinari]

In this case, I would propose we rename the prop focused activated. It matches perfectly (for the date range picker use case) with the terminology used in https://material.io/design/interaction/states.html#activated:

Activated states indicate which item from a set of options is currently being viewed.

It's also less confusing as it's not about the DOM focus, nor the aria focus.

---

@ameem91 What's your use case for the prop?

[filipesperandio]

Thanks for reporting this @ameem91, I was pulling my hair off. I know this is released, but I am still on an old build.
Got an workaround by making focused the inverse of disabled 👍