mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.63k stars 32.22k forks source link

[Masonry] ResizeObserver loop limit exceeded #36909

Closed SanNic20 closed 1 year ago

SanNic20 commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

Cannot be reproduced in a CodeSandbox or StackBlitz, because the virtual browser does not display the error.

Steps:

  1. Create React App : npx create-react-app masonry-app
  2. Install @mui/lab packages : npm install @mui/lab @mui/material @emotion/react @emotion/styled
  3. Edit the file src/App.js with the following code
import "./App.css";

import { useState } from 'react';
import { Box } from "@mui/material";
import Masonry from "@mui/lab/Masonry";
import { LoadingButton } from '@mui/lab';
import Paper from "@mui/material/Paper";
import { styled } from "@mui/material/styles";

const Item = styled(Paper)(({ theme }) => ({
  backgroundColor: theme.palette.mode === "dark" ? "#1A2027" : "#fff",
  ...theme.typography.body2,
  padding: theme.spacing(0.5),
  textAlign: "center",
  color: theme.palette.text.secondary,
}));

function App() {

const [heights, setheights] = useState([150, 30, 90, 70, 110, 150, 130, 80, 50, 90, 100, 150, 30, 50, 80]);

const addItem = () => {
  const updatedItems = [...heights];
  updatedItems.push(300);
  setheights(updatedItems);
};

  return (
      <>
      <Masonry
        columns={{ xs: 2, sm: 3, md: 4, lg: 5, xl: 6 }}
        spacing={2}
      >
        {heights.map((height, index) => (
          <Item key={index} sx={{ height }}>
            {index + 1}
          </Item>
        ))}
      </Masonry>
      <LoadingButton id="loadMore" size="large" variant="contained" onClick={addItem}>
        Add Item
      </LoadingButton>
      </>
  );
}

export default App;
  1. Run the app : npm start

Current behavior 😯

When we refresh the page, we get message error Compiled with problems: ResizeObserver loop limit exceeded at http://127.0.0.1:3000/static/js/bundle.js

If I use this same code in a larger project, I have the same problem, but the error remains displayed.

Also, this happen with Chrome and Edge, not tested on others.

Expected behavior 🤔

There should be no errors

Context 🔦

Note: If I dynamically add an element in the Masonry, I get the error again. Apparently, this problem occurs when recalculating the width and height of the Masonry.

I noticed too, that if I put a 100% width on the objects returned by the Masonry, it is worse, it creates a loop with this error until memory saturation.

Unfortunately I don't have a solution to this problem :( @hbjORbj : Could you may have a look please ? I hope someone can help me.

Your environment 🌎

npx @mui/envinfo ``` System: OS: Linux 5.10 Debian GNU/Linux 11 (bullseye) 11 (bullseye) Binaries: Node: 16.19.1 - /usr/bin/node Yarn: Not Found npm: 8.19.3 - /usr/bin/npm Browsers: Chrome: Not Found Firefox: Not Found npmPackages: @emotion/react: ^11.10.6 => 11.10.6 @emotion/styled: ^11.10.6 => 11.10.6 @mui/base: 5.0.0-alpha.125 @mui/core-downloads-tracker: 5.12.0 @mui/lab: ^5.0.0-alpha.126 => 5.0.0-alpha.126 @mui/material: ^5.12.0 => 5.12.0 @mui/private-theming: 5.12.0 @mui/styled-engine: 5.12.0 @mui/system: 5.12.0 @mui/types: 7.2.4 @mui/utils: 5.12.0 @types/react: 18.0.35 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: 4.9.5 ```
dpoehls commented 1 year ago

FYI - I am seeing the same error (currently using 5.11.15).

SanNic20 commented 1 year ago

@hbjORbj : I would also like to add that the issue also occurs with SSR. There is no visible message, but apparently the loop fills the memory, then crashes the application.

SanNic20 commented 1 year ago

@hbjORbj : I don't want to stress you, but do you think you can work on this issue over the next few days? (just to get an idea) If not, in the meantime, do you know if I can use a previous version that would not have this bug? Because this issue is very penalizing to develop an app which contains this component.

Last thing, do you know when this component will come out of the Lab mode?

Thank you in advance for your feedback.

ImOnMars commented 1 year ago

Getting the same issue here

Voraque commented 1 year ago

I'm experiencing the same error (mui 5.11.16, react 18.2.0).

SanNic20 commented 1 year ago

@hbjORbj @gzrae @oliviertassinari : Hi guys, any news about this issue ? Currently the Masonry component has become unusable in dev mode. This is very frustrating. Could you please take a look ?

Apparently, the problem is also on older versions. Let me explain: I have an old project with the Masonry component under React 17.0.2 and @mui/lab 5.0.0-alpha.84 and @mui/material 5.10.9. Everything works fine, but if I delete package-lock.json and the node_modules directory, and then reinstall packages with npm install, I now get the "ResizeObserver loop limit exceeded" error too, even though it has to reinstall the same versions. This makes me say that the problem seems to come from one of your dependent packages which has been updated.

Your help or any feedback will be very appreciated please. Thank you in advance.

aidan-mundy commented 1 year ago

Also experiencing this issue. Using chrome, mui/material 5.12.2, mui/lab 5.0.0-alpha.128, and react 18.2.0

It is more likely to occur when I have 1 or more useEffect on a page.

StanLi-TomTom commented 1 year ago

Also experiencing this issue. Using chrome, mui/material 5.12.2, mui/lab 5.0.0-alpha.128, and react 18.1.0

SanNic20 commented 1 year ago

Hello everybody, while waiting that the MUI team decides to fix or simply to answer my messages, here is a piece of code which makes temporarily the trick and that does not block you in your development.

useEffect(() => {
  window.addEventListener('error', e => {
      if (e.message === 'ResizeObserver loop limit exceeded') {
          const resizeObserverErrDiv = document.getElementById(
              'webpack-dev-server-client-overlay-div'
          );
          const resizeObserverErr = document.getElementById(
              'webpack-dev-server-client-overlay'
          );
          if (resizeObserverErr) {
              resizeObserverErr.setAttribute('style', 'display: none');
          }
          if (resizeObserverErrDiv) {
              resizeObserverErrDiv.setAttribute('style', 'display: none');
          }
      }
  });
}, []);
SanNic20 commented 1 year ago

Hi @hbjORbj @oliviertassinari @mnajdova This issue is really annoying. Is Masonry component still maintained? I see that other people also have some open issue with Masonry and nobody answers it. Thanks for your feedback.

mnajdova commented 1 year ago

@hbjORbj can you look into this soon?

hbjORbj commented 1 year ago

I will look into this today.

hbjORbj commented 1 year ago

@SanNic20 I am aware that you reported multiple issues with Masonry component, and I appreciate it. Still, I will reduce the scope of this issue to "ResizeObserver loop limit exceeded error is thrown when dynamically updating masonry children or refreshing the browser.".

I have opened a PR for the fix here

SanNic20 commented 1 year ago

@hbjORbj Thank you very much and sorry for my insistence.... Last thing, do you know when this component will come out of the Lab mode? Best regards

mnajdova commented 1 year ago

Have we considered https://github.com/DevExpress/testcafe/issues/4857#issuecomment-598775956? Adding a request animation frame can cause rendering delays. I feel like this should be a warning, instead of an error, but that is a different discussion. Is anything actually broken with the component when the error gets thrown?

hbjORbj commented 1 year ago

I think that's a valid concern. According to this (https://stackoverflow.com/a/50387233), we can ignore the error. The functionality doesn't get affected by the error.

mnajdova commented 1 year ago

I think that's a valid concern. According to this (https://stackoverflow.com/a/50387233), we can ignore the error. The functionality doesn't get affected by the error.

I would be curious to hear what the community thinks. If we decide to go with this road, we will need to have this documented somewhere on the Masonry page.

stevencreaney commented 1 year ago

It is not a good experience for us having this error thrown up. Dev Tools will constantly throw this to the UI, and Application Insights picks it up and logs it, so it's not really 'ignorable'.

SanNic20 commented 1 year ago

@mnajdova @hbjORbj : First I fully agree with @stevencreaney. Then, I think that it would be necessary to make some tests with SSR, because it seems that can impact it too, but I did not go in depth with those tests.

hbjORbj commented 1 year ago

MUI X DataGrid went with the same solution though. Ref: https://github.com/mui/mui-x/pull/8744 Should we proceed with merging the PR? @mnajdova

SanNic20 commented 1 year ago

@mnajdova : any news ???

kurtfm commented 1 year ago

I just switched to using ImageList with the 'masonry' variant... I know maybe not exactly the same, but does not have this issue.

mnajdova commented 1 year ago

MUI X DataGrid went with the same solution though. Ref: https://github.com/mui/mui-x/pull/8744 Should we proceed with merging the PR? @mnajdova

Approved, @hbjORbj left just one comment for clarifying the need for the new line

icon2341 commented 1 year ago

@hbjORbj have you had a chance to look at your pr?

roughpandaz commented 1 year ago

@SanNic20 will this still pop up in production?

18239388662 commented 5 months ago

Everyone, I have solved this problem, But he only focuses on my project The solution is to first copy out Mui/lab/Masonry. Remove ResizeObserver, Directly add the handleResize function in useEffect If there are images, add the Image load event to the handleResize function, and when triggered, call the handleResize function again @hbjORbj You can refer to it, although it may be a bit complicated, I still hope to package it into a universal one


const imgs = child.querySelector('img'); 

if (imgs && !imgs.dataset.loads){ 
  imgs.dataset.loads = 'true'; 
  imgs.onload = () => { handleResize(masonryChildren); } 
} 

React.useEffect(() => { 

    handleResize(masonryRef.current.childNodes); 

}, [columns, spacing, children]);