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

[material-ui][TextareaAutosize] Temporarily disconnect ResizeObserver to avoid loop error #44540

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mj12albert
Copy link
Member

@mj12albert mj12albert commented Nov 25, 2024

Fixes #43718
Fixes #44140

Demo: https://codesandbox.io/p/sandbox/immutable-architecture-nslxvr?file=%2Fsrc%2FApp.js%3A33%2C1 (forked from the original repro)

This was very hard to repro locally, but I believe the "loop" is caused when we change the height of the element in the RO callback, while it is still being observed, which fires the callback again (and again…)

The fix is to un-observe the element, manipulate the height then re-observe one frame later, instead of manipulating the height one frame later suggested here (or here).

@mj12albert mj12albert added the component: TextareaAutosize The React component. label Nov 25, 2024
@mui-bot
Copy link

mui-bot commented Nov 25, 2024

Netlify deploy preview

https://deploy-preview-44540--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 5196ed1

@mj12albert mj12albert force-pushed the fix/textarea-ro-loop branch 5 times, most recently from b152a03 to 72a6aaa Compare November 25, 2024 10:06
@mj12albert mj12albert changed the title [TextareaAutosize] Fix RO error [TextareaAutosize] Temporarily un-observe element when resizing Nov 25, 2024
@mj12albert mj12albert marked this pull request as ready for review November 25, 2024 11:59
@mj12albert mj12albert requested review from mnajdova, michaldudak and aarongarciah and removed request for michaldudak November 25, 2024 12:42
@@ -36,15 +36,20 @@ type TextareaStyles = {
overflowing: boolean;
};

function isObjectEmpty(object: TextareaStyles) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@mj12albert mj12albert force-pushed the fix/textarea-ro-loop branch 2 times, most recently from 5b99446 to 3a7aaf2 Compare November 25, 2024 13:35
@aarongarciah aarongarciah added the package: material-ui Specific to @mui/material label Nov 25, 2024
@aarongarciah aarongarciah changed the title [TextareaAutosize] Temporarily un-observe element when resizing [material-ui][TextareaAutosize] Temporarily un-observe element when resizing Nov 25, 2024
@aarongarciah
Copy link
Member

@mj12albert since this was hard to reproduce and might have some edge cases, can you add a comment in the related issue (#43718) to see if some users can test with this PR's version? They'd need to install https://pkg.csb.dev/mui/material-ui/commit/3a7aaf21/@mui/material

@mj12albert
Copy link
Member Author

can you add a comment in the related issue

Added ~ #43718 (comment)

@mj12albert mj12albert force-pushed the fix/textarea-ro-loop branch 6 times, most recently from e2af729 to cdaa830 Compare December 3, 2024 12:15
@mj12albert mj12albert force-pushed the fix/textarea-ro-loop branch 2 times, most recently from 44de4da to b571946 Compare December 5, 2024 07:12
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 9, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 10, 2024
@mj12albert mj12albert force-pushed the fix/textarea-ro-loop branch 3 times, most recently from d84fef6 to 239b54b Compare December 13, 2024 04:22
@mj12albert mj12albert added the regression A bug, but worse label Dec 23, 2024
@mj12albert mj12albert changed the title [material-ui][TextareaAutosize] Temporarily un-observe element when resizing [material-ui][TextareaAutosize] Temporarily disconnect ResizeObserver to avoid loop error Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: TextareaAutosize The React component. package: material-ui Specific to @mui/material regression A bug, but worse
Projects
None yet
3 participants