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] Simplify logic and add test #38728

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Aug 30, 2023

A quick continuation of #37135. I was triggered because I saw 3 PRs that fix bugs without tests on the same component:

  1. [TextareaAutosize] Fix crash when used with React 18 & Suspense #33238.
  2. [TextareaAutosize] Fix warnings for too many renders in React 18 #33253
  3. [TextareaAutosize] Fix 166ms resize update lag #37135. I have tried to add tests on this one, but the problem is that in Karma, the browser gets confused, It triggers ResizeObserver loop completed with undelivered notifications but it's an hallucination https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver#observation_errors. More details in ResizeObserver#observe() firing the callback immediately results in infinite loop WICG/resize-observer#38, also faced in ResizeObserver loop error in react error overlay in dev mode mui-x#8733.

I'm also removing these if (inputRef.current) { defensive checks. Sebastian was asking the right question in #33238 (comment). As far as I know, the origin is React 17 facebook/react#17925 which doesn't run the effect cleanup synchronously anymore.

@mnajdova could we make the same change in https://github.com/mui/material-ui/pull/33277/files, moving to a layout effect? Thanks


if (resizeObserver) {
resizeObserver.disconnect();
}
};
});
}, [getUpdatedState]);
Copy link
Member Author

@oliviertassinari oliviertassinari Aug 30, 2023

Choose a reason for hiding this comment

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

Missing dependencies, the listeners were recreated at each render.

@mui-bot
Copy link

mui-bot commented Aug 30, 2023

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against ec8b5e6

@oliviertassinari oliviertassinari force-pushed the TextareaAutosize-simplify branch 4 times, most recently from f737871 to 8b6edaa Compare September 16, 2023 20:24
@oliviertassinari oliviertassinari enabled auto-merge (squash) September 18, 2023 20:08
@oliviertassinari oliviertassinari merged commit 5fc9508 into mui:master Sep 18, 2023
9 checks passed
@oliviertassinari oliviertassinari deleted the TextareaAutosize-simplify branch September 18, 2023 20:09
christophermorin pushed a commit to christophermorin/material-ui that referenced this pull request Sep 21, 2023
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 7, 2023

Released in v5.14.11 on Sep 26, 2023.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants