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

[data grid] infinite loop occurs under certain circumstances #13230

Closed
layerok opened this issue May 23, 2024 · 10 comments
Closed

[data grid] infinite loop occurs under certain circumstances #13230

layerok opened this issue May 23, 2024 · 10 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Rendering layout Related to the data grid Rendering engine support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@layerok
Copy link
Contributor

layerok commented May 23, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/p/devbox/frosty-faraday-lt52lc?file=%2Fsrc%2FApp.tsx%3A135%2C24

Steps:

  1. You don't have to do anything, just open the console and see that the error is thrown indefinitely, don't worry I limited it to maximum of 50 cycles so that your browser doesn't crush

Current behavior

Infinite loop

Expected behavior

No infinite loop

Context

I want to...

  1. calculate page size based on data grid dimensions.
  2. preserve the pagination model in the URL.

I have successfully completed everything listed above.
But there is one problem that occurs under certain circumstances. Infinite loop.
The circumstances are as follows.

  • an error is thrown while rendering data grid component
  • a loader has been defined for one of the currently matched routes
  • react-router navigate function is called inside the viewportInnerSizeChange event listener

If all of the circumstances are true, the infinite loop is guaranteed.
If one of the circumstances isn't true, the infinite loop doesn't occur.

This what I understood what is happening

  1. An error is thrown while rendering the data grid
  2. The error is caught by the react-router error boundary
  3. The error boundary is rendered.
  4. The viewportInnerSizeChange event is emitted in useEffect in the useGridDimensions hook
  5. Search params are updated in my custom viewportInnerSizeChange listener
  6. The error boundary is unmounted because the location was changed due to the search params change
  7. The data grid renders again
  8. All of above steps are repeated indefinitely

What's weird is why the viewportInnerSizeChange event is emitted after the error boundary is rendered.
I'm guessing this useEffect might be causing the error.

  useEnhancedEffect(() => {
    if (savedSize) {
      updateDimensions();
      apiRef.current.publishEvent('debouncedResize', rootDimensionsRef.current!);
    }
  }, [apiRef, savedSize, updateDimensions]);

Your environment

No response

Search keywords: react router infinite loop loader viewportInnerSizeChange navigate
Order ID: 74777

@layerok layerok added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 23, 2024
@layerok layerok changed the title [data grid] infinite loop occurs under certain circumstances when an error is thrown while data grid rendering [data grid] infinite loop occurs under certain circumstances May 23, 2024
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label May 23, 2024
@michelengelen
Copy link
Member

Hey @layerok ... thanks for opening this issue.
I see that you already came up with a solution and opened a PR.
I did add some reviewers to that so it gets picked up by the team. 👍🏼

@michelengelen michelengelen added feature: Rendering layout Related to the data grid Rendering engine support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 24, 2024
@romgrk
Copy link
Contributor

romgrk commented May 24, 2024

IIUC, to trigger this issue, there needs to be an error thrown from inside the grid rendering code, which in this case is user code, right?

@layerok
Copy link
Contributor Author

layerok commented May 24, 2024

Yes, in this case the error came from user code, but that doesn't matter. The error could have come from the library code. For example, an error is thrown if any translation string is missing.

@romgrk
Copy link
Contributor

romgrk commented May 25, 2024

I think the issue is the error, not the infinite loop. There shouldn't be an error thrown from the translation code, everything should be complete. I would be ok with adding an error boundary to catch the error and fail gracefully, but I don't think we should start adding code to the rest of the logic because "anything might throw". It just complicates the logic too much for what it brings.

@layerok
Copy link
Contributor Author

layerok commented May 26, 2024

First of all, "everything should be complete" for me is a very optimistic assumtion in this situation. If this assumtion is not true, the user's browser will crush. I don't want that.
For example, my browser crushed because an unexpected error was thrown from inside library code.

Secondly, I don't think there is only one cause to this issue. This issue won't reproduce if one of 3 conditions is absent. I don't know for sure if this is mui or react-router bug.
It might be a bug in react-router or in both libraries simultaneously.
I created the issue in mui, because I had to start somewhere.

You said that you would be ok to wrap datagrid in an error boundary to fail gracefully. I'm not sure if this will help, react-router already wraps every route in an error boundary, but it does not help.

Right now I'm trying to create a minimal reproducible example to understand what the problem is. I'm creating my custom react-router and datagrid implementations, removing everything that isn't related to the problem.

@layerok
Copy link
Contributor Author

layerok commented May 26, 2024

I found one interesting detail. An error that occurs while rendering a cell does not result in the infinite loop, unlike errors within the header and header filter cells.

@layerok
Copy link
Contributor Author

layerok commented May 26, 2024

ok, I figured it out, it turns out that calling the router.navigate function in useEffect schedules update in a microtask that cannot be cancelled.
What I mean is when you are doing this

useEffect(() => {
  router.navigate(...)
}, []);

You are essentially doing this

useEffect(() => {
  setState({...});
  // react-router resolves loaders
  Promise.resolve().then(() => {
    // state update in a microtask
    setState({...});
  })
}, [])

To fix the infinite loop we need to somehow cancel this microtask. Unfortunately there is no way to cancel pending navigation in react-router. But we can workaround this problem by wrapping router.navigate call in setTimeout. This way we will be able to abort the navigation before it even starts.

useEffect(() => {
  let timeout = setTimeout(() => {
      router.navigate(...);
  })
  return () => clearInterval(timeout);
}, [])

It also would be nice if MUI didn't emit events inside layout or passive effect, thus this problem wouldn't exist at all. But I guess this will be massive rework of the library.

@layerok
Copy link
Contributor Author

layerok commented May 27, 2024

Finally, I've created a minimally reproducible example as possible
Now it all makes sense to me. Why removing loaders can fix the infinite loop. Why react-router completed navigation while error boundary was rendering.

That's roughly what's going on

  1. HomePage first renders
  2. NoSsr renders but doesn't render RenderSomethingThatThrows immediately
  3. HomePage layout effects run
  4. router starts navigation, firing first setState
  5. HomePage rerenders
  6. NoSsr renders the RenderSomethingThatThrows component
  7. The error is caught by an error boundary
  8. layout effects effect run
  9. router completes navigation in the microtask, after waiting for loaders to resolve, firing second setState
  10. All about steps from step 5 are repeated indefinitely
const HomePage = () => {
  const navigate = useNavigate();
  useEffect(() => {
    //const timeout = setTimeout(() => {
      navigate({
        search: "?page=2"
      })
    //})
    return () => {
      //clearTimeout(timeout)
    }
  }, [navigate]);

  return (
    <NoSsr>
      <RenderSomethingThatThrows/>
    </NoSsr>
  );
}

const router = createBrowserRouter([
  {
    path: "/",
    element: <HomePage />,
    loader: async () => {
      // remove this loader to prevent the infinite loop
      return null;
    },
  },
]);

function App() {
  return <RouterProvider router={router} />;
}

@layerok
Copy link
Contributor Author

layerok commented May 27, 2024

this is not a MUI issue

@layerok layerok closed this as completed May 27, 2024
Copy link

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@layerok: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Rendering layout Related to the data grid Rendering engine support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants