-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[TextareaAutosize] Fix warnings for too many renders in React 18 #33253
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import * as React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import { flushSync } from 'react-dom'; | ||
import { | ||
unstable_debounce as debounce, | ||
unstable_useForkRef as useForkRef, | ||
|
@@ -27,6 +28,10 @@ const styles = { | |
}, | ||
}; | ||
|
||
function isEmpty(obj) { | ||
return obj === undefined || obj === null || Object.keys(obj).length === 0; | ||
} | ||
|
||
const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref) { | ||
const { onChange, maxRows, minRows = 1, style, value, ...other } = props; | ||
|
||
|
@@ -37,14 +42,14 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref) | |
const renders = React.useRef(0); | ||
const [state, setState] = React.useState({}); | ||
|
||
const syncHeight = React.useCallback(() => { | ||
const getUpdatedState = React.useCallback(() => { | ||
const input = inputRef.current; | ||
const containerWindow = ownerWindow(input); | ||
const computedStyle = containerWindow.getComputedStyle(input); | ||
|
||
// If input's width is shrunk and it's not visible, don't sync height. | ||
if (computedStyle.width === '0px') { | ||
return; | ||
return {}; | ||
} | ||
|
||
const inputShallow = shadowRef.current; | ||
|
@@ -86,41 +91,72 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref) | |
const outerHeightStyle = outerHeight + (boxSizing === 'border-box' ? padding + border : 0); | ||
const overflow = Math.abs(outerHeight - innerHeight) <= 1; | ||
|
||
setState((prevState) => { | ||
// Need a large enough difference to update the height. | ||
// This prevents infinite rendering loop. | ||
if ( | ||
renders.current < 20 && | ||
((outerHeightStyle > 0 && | ||
Math.abs((prevState.outerHeightStyle || 0) - outerHeightStyle) > 1) || | ||
prevState.overflow !== overflow) | ||
) { | ||
renders.current += 1; | ||
return { | ||
overflow, | ||
outerHeightStyle, | ||
}; | ||
} | ||
return { outerHeightStyle, overflow }; | ||
}, [maxRows, minRows, props.placeholder]); | ||
|
||
if (process.env.NODE_ENV !== 'production') { | ||
if (renders.current === 20) { | ||
console.error( | ||
[ | ||
'MUI: Too many re-renders. The layout is unstable.', | ||
'TextareaAutosize limits the number of renders to prevent an infinite loop.', | ||
].join('\n'), | ||
); | ||
} | ||
const updateState = (prevState, newState) => { | ||
const { outerHeightStyle, overflow } = newState; | ||
// Need a large enough difference to update the height. | ||
// This prevents infinite rendering loop. | ||
if ( | ||
renders.current < 20 && | ||
((outerHeightStyle > 0 && | ||
Math.abs((prevState.outerHeightStyle || 0) - outerHeightStyle) > 1) || | ||
prevState.overflow !== overflow) | ||
) { | ||
renders.current += 1; | ||
return { | ||
overflow, | ||
outerHeightStyle, | ||
}; | ||
} | ||
if (process.env.NODE_ENV !== 'production') { | ||
if (renders.current === 20) { | ||
console.error( | ||
[ | ||
'MUI: Too many re-renders. The layout is unstable.', | ||
'TextareaAutosize limits the number of renders to prevent an infinite loop.', | ||
].join('\n'), | ||
); | ||
} | ||
} | ||
return prevState; | ||
}; | ||
|
||
const syncHeight = React.useCallback(() => { | ||
const newState = getUpdatedState(); | ||
|
||
if (isEmpty(newState)) { | ||
return; | ||
} | ||
|
||
return prevState; | ||
setState((prevState) => { | ||
return updateState(prevState, newState); | ||
}); | ||
}, [maxRows, minRows, props.placeholder]); | ||
}, [getUpdatedState]); | ||
|
||
const syncHeightWithFlushSycn = () => { | ||
const newState = getUpdatedState(); | ||
|
||
if (isEmpty(newState)) { | ||
return; | ||
} | ||
|
||
// In React 18, state updates in a ResizeObserver's callback are happening after the paint which causes flickering | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know you're nerd-sniping me because I'm pretty sure this should be fixed with some experimental features π On vacation now but I'll definitely take a look later because having to reach for flushSync every time you're in a ResizeObserver callback seems odd and something you often miss unless you've encountered this bug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha it's not intentional, but yes, I agree it's strange and should be fixed once someone is back from vacation π Just kidding, but really looks like a strange requirement and more importanly is not easy to capture the bugs or create reliable tests. |
||
// when doing some visual updates in it. Using flushSync ensures that the dom will be painted after the states updates happen | ||
// Related issue - https://github.com/facebook/react/issues/24331 | ||
// TODO: Do this only in the resize observer? | ||
flushSync(() => { | ||
setState((prevState) => { | ||
return updateState(prevState, newState); | ||
}); | ||
}); | ||
}; | ||
|
||
React.useEffect(() => { | ||
const handleResize = debounce(() => { | ||
renders.current = 0; | ||
syncHeight(); | ||
syncHeightWithFlushSycn(); | ||
}); | ||
const containerWindow = ownerWindow(inputRef.current); | ||
containerWindow.addEventListener('resize', handleResize); | ||
|
@@ -138,7 +174,7 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(props, ref) | |
resizeObserver.disconnect(); | ||
} | ||
}; | ||
}, [syncHeight]); | ||
}); | ||
|
||
useEnhancedEffect(() => { | ||
syncHeight(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically the same function as
syncHeight
, it is just usingflushSync
for updating the state. We cannot and shouldn't useflushSync
in all other cases where we are synching the height, so we are using the same method as before.