[material-ui][TextareaAutosize] Temporarily disconnect ResizeObserver to avoid loop error #44540
Conversation
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
from
e712092 to
e2d5825
Compare
Netlify deploy previewhttps://deploy-preview-44540--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
5 times, most recently
from
b152a03 to
72a6aaa
Compare
mj12albert
changed the title
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
from
72a6aaa to
d140f30
Compare
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
from
d140f30 to
044a802
Compare
mj12albert
requested review from
mnajdova,
michaldudak and
aarongarciah
and removed request for
michaldudak
| @@ -36,15 +36,20 @@ type TextareaStyles = { | |||
| overflowing: boolean; | |||
| }; | |||
|
|
|||
| function isObjectEmpty(object: TextareaStyles) { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
We can expose an util for this and share it in the different packages.
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
2 times, most recently
from
5b99446 to
3a7aaf2
Compare
aarongarciah
changed the title
|
@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 |
Added ~ #43718 (comment) |
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
6 times, most recently
from
e2af729 to
cdaa830
Compare
github-actions
bot
removed
the
PR: out-of-date
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
3 times, most recently
from
d84fef6 to
239b54b
Compare
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
from
239b54b to
5196ed1
Compare
mj12albert
changed the title
aarongarciah
force-pushed
the
fix/textarea-ro-loop
branch
from
5196ed1 to
1e81206
Compare
|
@mj12albert no responses in #43718 (comment) so let's try to move this forward and see if the reports stop. @mnajdova and @DiegoAndai can you review this? |
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
2 times, most recently
from
cbf8a4a to
9e4b8e9
Compare
Finally got one! 😅 |
mj12albert
force-pushed
the
fix/textarea-ro-loop
branch
from
9e4b8e9 to
91b919f
Compare
packages/mui-material/src/TextareaAutosize/TextareaAutosize.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| containerWindow.addEventListener('resize', debounceHandleResize); | ||
|
|
||
| let resizeObserver: ResizeObserver; | ||
|
|
||
| if (typeof ResizeObserver !== 'undefined') { | ||
| resizeObserver = new ResizeObserver( | ||
| process.env.NODE_ENV === 'test' ? rAFHandleResize : handleResize, |
There was a problem hiding this comment.
I wonder why we only usedrequestAnimationFrame for the test environment before this change 🤔
There was a problem hiding this comment.
Probably it triggers more frequently or consistently in tests envs?
If we actually used it though I suspect it would cause some jankiness: https://stackoverflow.com/questions/49384120/resizeobserver-loop-limit-exceeded#comment118281231_58701523
|
@DiegoAndai Hey, is this going to be added to v5 as well? Thanks |
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).