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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TextField] "Maximum update depth exceeded." in Internet Explorer 11 #17672

Closed
2 tasks done
bitoverflow opened this issue Oct 2, 2019 · 7 comments · Fixed by #19743
Closed
2 tasks done

[TextField] "Maximum update depth exceeded." in Internet Explorer 11 #17672

bitoverflow opened this issue Oct 2, 2019 · 7 comments · Fixed by #19743
Labels
bug 🐛 Something doesn't work component: TextareaAutosize The React component. good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@bitoverflow
Copy link

bitoverflow commented Oct 2, 2019

App crashes with "Maximum update depth exceeded" triggered by filling a TextField on Internet Explorer 11

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

When typing text in a Textfield, the app crashed when all visible space is filled up.
Only observed in IE11

Expected Behavior 🤔

The initial visible rows of the textField should grow until reaching rowsMax.
Works as expected in Safari and Chrome

Steps to Reproduce 🕹

https://stackblitz.com/edit/react-ts-6luqab

Steps:

  1. The Textfield need the following parameters rows, rowsMax, fullWidth, multiline
  2. Type text in textfield until crashing (fill all visible space)

Your Environment 🌎

Tech Version
Material-UI v4.5.0
React 16.10.0
React-Dom 16.10.0
Browser Internet Explorer 11 (tested in Browserstack)
@bitoverflow bitoverflow changed the title "Maximum update depth exceeded." triggered by TextField on Internet Explorer 11 [TextField] "Maximum update depth exceeded." in Internet Explorer 11 Oct 2, 2019
@adeelibr
Copy link
Contributor

adeelibr commented Oct 2, 2019

I can confirm this behavior. I will try to look into this 😄 Thank you for pointing this out.

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Oct 3, 2019
@oliviertassinari oliviertassinari added component: text field This is the name of the generic UI component, not the React module! component: TextareaAutosize The React component. labels Nov 30, 2019
@oliviertassinari
Copy link
Member

In this context, I would propose that we put protection in place to limit the number of rerenders, we don't want to see the UI crash because of a layout instability:

diff --git a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js
index d7040ddc1..52550922a 100644
--- a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js
+++ b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js
@@ -33,6 +33,7 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref)
   const inputRef = React.useRef(null);
   const handleRef = useForkRef(ref, inputRef);
   const shadowRef = React.useRef(null);
+  const renders = React.useRef(0);
   const [state, setState] = React.useState({});

   const syncHeight = React.useCallback(() => {
@@ -76,10 +77,12 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref)
       // Need a large enough different to update the height.
       // This prevents infinite rendering loop.
       if (
-        (outerHeightStyle > 0 &&
+        renders.current < 10 &&
+        ((outerHeightStyle > 0 &&
           Math.abs((prevState.outerHeightStyle || 0) - outerHeightStyle) > 1) ||
-        prevState.overflow !== overflow
+          prevState.overflow !== overflow)
       ) {
+        renders.current += 1;
         return {
           overflow,
           outerHeightStyle,
@@ -91,6 +94,8 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref)
   }, [rows, rowsMax, props.placeholder]);

   React.useEffect(() => {
+    renders.current = 0;
+
     const handleResize = debounce(() => {
       syncHeight();
     });
@@ -107,6 +112,8 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref)
   });

   const handleChange = event => {
+    renders.current = 0;
+
     if (!isControlled) {
       syncHeight();
     }

Maybe, we would want to trigger a warning and a test case too.

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. and removed component: text field This is the name of the generic UI component, not the React module! labels Dec 5, 2019
@captain-yossarian
Copy link
Contributor

@adeelibr If you don't mind I can pick it up

@adeelibr
Copy link
Contributor

@SerhiiBilyk sure go ahead :)

@captain-yossarian
Copy link
Contributor

@adeelibr unfortunately I'm unable to run IE on my laptop :( So feel free to pick it up

@st3ffane
Copy link

I tried version 4.7.2 on Chrome 79.0.3945.130 on Windows 10 and reproduced this issue.
Seems to be better when adding "box-sizing: border-box" to the .inputbar-input class.

@SofianeDjellouli
Copy link
Contributor

Hi, I'll be working on this if that's OK with you guys.

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: TextareaAutosize The React component. good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
6 participants