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

[TextareaAutosize] Improve implementation #40789

Merged
28 changes: 0 additions & 28 deletions packages/mui-base/src/TextareaAutosize/TextareaAutosize.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
createMount,
createRenderer,
fireEvent,
strictModeDoubleLoggingSuppressed,
} from '@mui-internal/test-utils';
import { TextareaAutosize } from '@mui/base/TextareaAutosize';

Expand Down Expand Up @@ -458,32 +457,5 @@ describe('<TextareaAutosize />', () => {
// the input should be 2 lines
expect(input.style).to.have.property('height', `${lineHeight * 2}px`);
});

describe('warnings', () => {
it('warns if layout is unstable but not crash', () => {
const { container, forceUpdate } = render(<TextareaAutosize maxRows={3} />);
const input = container.querySelector<HTMLTextAreaElement>('textarea[aria-hidden=null]')!;
const shadow = container.querySelector('textarea[aria-hidden=true]')!;
let index = 0;
setLayout(input, shadow, {
getComputedStyle: {
boxSizing: 'content-box',
},
scrollHeight: 100,
lineHeight: () => {
index += 1;
return index;
},
});

expect(() => {
forceUpdate();
}).toErrorDev([
'MUI: Too many re-renders.',
!strictModeDoubleLoggingSuppressed && 'MUI: Too many re-renders.',
!strictModeDoubleLoggingSuppressed && 'MUI: Too many re-renders.',
]);
});
});
});
});
102 changes: 20 additions & 82 deletions packages/mui-base/src/TextareaAutosize/TextareaAutosize.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use client';
import * as React from 'react';
import PropTypes from 'prop-types';
import * as ReactDOM from 'react-dom';
import {
unstable_debounce as debounce,
unstable_useForkRef as useForkRef,
Expand All @@ -10,11 +9,6 @@ import {
} from '@mui/utils';
import { TextareaAutosizeProps } from './TextareaAutosize.types';

type State = {
outerHeightStyle: number;
overflow?: boolean | undefined;
};

function getStyleValue(value: string) {
return parseInt(value, 10) || 0;
}
Expand All @@ -37,12 +31,17 @@ const styles: {
},
};

function isEmpty(obj: State) {
type TextareaStyles = {
outerHeightStyle: number;
overflowing: boolean;
};

function isEmpty(obj: TextareaStyles) {
return (
obj === undefined ||
obj === null ||
Object.keys(obj).length === 0 ||
(obj.outerHeightStyle === 0 && !obj.overflow)
(obj.outerHeightStyle === 0 && !obj.overflowing)
);
}

Expand All @@ -64,15 +63,11 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(
const { onChange, maxRows, minRows = 1, style, value, ...other } = props;

const { current: isControlled } = React.useRef(value != null);
const inputRef = React.useRef<HTMLInputElement>(null);
const inputRef = React.useRef<HTMLTextAreaElement>(null);
const handleRef = useForkRef(forwardedRef, inputRef);
const shadowRef = React.useRef<HTMLTextAreaElement>(null);
const renders = React.useRef(0);
const [state, setState] = React.useState<State>({
outerHeightStyle: 0,
});

const getUpdatedState = React.useCallback(() => {
const calculateTextareaStyles = React.useCallback(() => {
const input = inputRef.current!;

const containerWindow = ownerWindow(input);
Expand All @@ -82,6 +77,7 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(
if (computedStyle.width === '0px') {
return {
outerHeightStyle: 0,
overflowing: false,
};
}

Expand Down Expand Up @@ -122,71 +118,26 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(

// Take the box sizing into account for applying this value as a style.
const outerHeightStyle = outerHeight + (boxSizing === 'border-box' ? padding + border : 0);
const overflow = Math.abs(outerHeight - innerHeight) <= 1;
const overflowing = Math.abs(outerHeight - innerHeight) <= 1;

return { outerHeightStyle, overflow };
return { outerHeightStyle, overflowing };
}, [maxRows, minRows, props.placeholder]);

const updateState = (prevState: State, newState: State) => {
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) {
Copy link
Member

@oliviertassinari oliviertassinari Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we still need a failsafe for unstable layouts (unrelated to React). This happens when the CSS values are unstable, this warning is also meant to invite developers to create issues so we can debug and fix the logic. Typically you would see the height flicker up and down as you type in the input. Or freeze the app because the resize observer logic run in a loop.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for unstable layouts (unrelated to React). This happens when the CSS values are unstable,

Can you provide reproductions of layout instability not related to React? I'm open to debugging it.

this warning is also meant to invite developers to create issues so we can debug and fix the logic

Sure, but since we're not relying on React state anymore, there's no need for warnings about excessive re-renders caused by state.

Typically you would see the height flicker up and down as you type in the input. Or freeze the app because the resize observer logic run in a loop.

Again, can you provide reproductions using this PR build?

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();
const textareaStyles = calculateTextareaStyles();

if (isEmpty(newState)) {
if (isEmpty(textareaStyles)) {
return;
}

setState((prevState) => updateState(prevState, newState));
}, [getUpdatedState]);
const input = inputRef.current!;
input.style.setProperty('height', `${textareaStyles.outerHeightStyle}px`);
input.style.setProperty('overflow', textareaStyles.overflowing ? 'hidden' : null);
ZeeshanTamboli marked this conversation as resolved.
Show resolved Hide resolved
}, [calculateTextareaStyles]);

useEnhancedEffect(() => {
const syncHeightWithFlushSync = () => {
const newState = getUpdatedState();

if (isEmpty(newState)) {
return;
}

// In React 18, state updates in a ResizeObserver's callback are happening after
// the paint, this leads to an infinite rendering.
//
// Using flushSync ensures that the states is updated before the next pain.
// Related issue - https://github.com/facebook/react/issues/24331
ReactDOM.flushSync(() => {
setState((prevState) => updateState(prevState, newState));
});
};

const handleResize = () => {
renders.current = 0;
syncHeightWithFlushSync();
syncHeight();
};
// Workaround a "ResizeObserver loop completed with undelivered notifications" error
// in test.
Expand Down Expand Up @@ -222,19 +173,13 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(
resizeObserver.disconnect();
}
};
}, [getUpdatedState]);
}, [calculateTextareaStyles, syncHeight]);

useEnhancedEffect(() => {
syncHeight();
});

React.useEffect(() => {
renders.current = 0;
}, [value]);

const handleChange = (event: React.ChangeEvent<HTMLTextAreaElement>) => {
renders.current = 0;

if (!isControlled) {
syncHeight();
}
Expand All @@ -252,13 +197,6 @@ const TextareaAutosize = React.forwardRef(function TextareaAutosize(
ref={handleRef}
// Apply the rows prop to get a "correct" first SSR paint
rows={minRows as number}
style={{
height: state.outerHeightStyle,
// Need a large enough difference to allow scrolling.
// This prevents infinite rendering loop.
overflow: state.overflow ? 'hidden' : undefined,
...style,
}}
{...other}
/>
<textarea
Expand Down
Loading