From 8b99f6fcb260983dca6d7de167d5ca988aad2e4a Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Wed, 19 Feb 2020 14:02:37 +0100 Subject: [PATCH] bound max number of renders --- .../src/TextareaAutosize/TextareaAutosize.js | 28 ++++++++++++-- .../TextareaAutosize/TextareaAutosize.test.js | 37 ++++++++++++++++++- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js index 3bf46a5a6113df..d39f89e3c39131 100644 --- a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js +++ b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js @@ -35,6 +35,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(() => { @@ -72,29 +73,42 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref) // Take the box sizing into account for applying this value as a style. const outerHeightStyle = outerHeight + (boxSizing === 'border-box' ? padding + border : 0); - const diff = Math.abs(outerHeight - innerHeight); - const overflow = diff <= 1 || diff === innerHeight; + const overflow = Math.abs(outerHeight - innerHeight) <= 1; setState(prevState => { // Need a large enough difference to update the height. // This prevents infinite rendering loop. if ( - (outerHeightStyle > 0 && + renders.current < 20 && + ((outerHeightStyle > 0 && Math.abs((prevState.outerHeightStyle || 0) - outerHeightStyle) > 1) || - prevState.overflow !== overflow + prevState.overflow !== overflow) ) { + renders.current += 1; return { overflow, outerHeightStyle, }; } + if (process.env.NODE_ENV !== 'production') { + if (renders.current === 20) { + console.error( + [ + 'Material-UI: too many re-renders. The layout is unstable.', + 'TextareaAutosize limits the number of renders to prevent an infinite loop.', + ].join('\n'), + ); + } + } + return prevState; }); }, [rowsMax, rowsMin, props.placeholder]); React.useEffect(() => { const handleResize = debounce(() => { + renders.current = 0; syncHeight(); }); @@ -109,7 +123,13 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref) syncHeight(); }); + React.useEffect(() => { + renders.current = 0; + }, [value]); + const handleChange = event => { + renders.current = 0; + if (!isControlled) { syncHeight(); } diff --git a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.test.js b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.test.js index d2bffa98d82707..b3c8c5e65f2197 100644 --- a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.test.js +++ b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.test.js @@ -3,6 +3,7 @@ import { assert } from 'chai'; import sinon, { spy, stub, useFakeTimers } from 'sinon'; import { createMount } from '@material-ui/core/test-utils'; import describeConformance from '@material-ui/core/test-utils/describeConformance'; +import consoleErrorMock from 'test/utils/consoleErrorMock'; import TextareaAutosize from './TextareaAutosize'; function getStyle(wrapper) { @@ -38,7 +39,9 @@ describe('', () => { const getComputedStyleStub = {}; - function setLayout(wrapper, { getComputedStyle, scrollHeight, lineHeight }) { + function setLayout(wrapper, { getComputedStyle, scrollHeight, lineHeight: lineHeightArg }) { + const lineHeight = typeof lineHeightArg === 'function' ? lineHeightArg : () => lineHeightArg; + const input = wrapper .find('textarea') .at(0) @@ -53,7 +56,7 @@ describe('', () => { let index = 0; stub(shadow, 'scrollHeight').get(() => { index += 1; - return index % 2 === 1 ? scrollHeight : lineHeight; + return index % 2 === 1 ? scrollHeight : lineHeight(); }); } @@ -237,5 +240,35 @@ describe('', () => { wrapper.update(); assert.deepEqual(getStyle(wrapper), { height: lineHeight * 2, overflow: null }); }); + + describe('warnings', () => { + before(() => { + consoleErrorMock.spy(); + }); + + after(() => { + consoleErrorMock.reset(); + }); + + it('warns if layout is unstable but not crash', () => { + const wrapper = mount(); + let index = 0; + setLayout(wrapper, { + getComputedStyle: { + 'box-sizing': 'content-box', + }, + scrollHeight: 100, + lineHeight: () => { + index += 1; + return 15 + index; + }, + }); + wrapper.setProps(); + wrapper.update(); + + assert.strictEqual(consoleErrorMock.callCount(), 3); + assert.include(consoleErrorMock.args()[0][0], 'Material-UI: too many re-renders.'); + }); + }); }); });