mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.79k stars 1.12k forks source link

[data grid] Grouping Col Def with a flex and pinned columns causes "Maximum update depth exceeded" #12851

Open cstephens-cni opened 2 weeks ago

cstephens-cni commented 2 weeks ago

Steps to reproduce

Link to live example: (required) https://codesandbox.io/p/sandbox/ecstatic-sid-4xygr9?file=%2Fsrc%2FDemo.tsx%3A47%2C1 Steps:

  1. fails on load
  2. remove flex from grouping column and it will load
  3. can also remove pinned col to have it load

https://github.com/mui/mui-x/issues/12584#issuecomment-2061773641 reported a similar issue. With different pinned columns

Current behavior

Error thrown

Expected behavior

grid to load and pinned grouped column to use flex

Context

Pinning a grouped column that can flex so we can use it as the defining col of that row.

Your environment

npx @mui/envinfo ``` System: OS: macOS 14.4.1 Binaries: Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node npm: 9.6.7 - ~/.nvm/versions/node/v18.17.1/bin/npm pnpm: Not Found Browsers: Chrome: 124.0.6367.60 Edge: 124.0.2478.51 Safari: 17.4.1 npmPackages: @emotion/react: 11.11.3 => 11.11.3 @emotion/styled: 11.11.0 => 11.11.0 @mui/base: ^5.0.0-beta.40 => 5.0.0-beta.40 @mui/core-downloads-tracker: 5.15.15 @mui/icons-material: ^5.15.15 => 5.15.15 @mui/lab: ^5.0.0-alpha.170 => 5.0.0-alpha.170 @mui/material: ^5.15.15 => 5.15.15 @mui/private-theming: 5.15.14 @mui/styled-engine: 5.15.14 @mui/system: ^5.15.15 => 5.15.15 @mui/types: 7.2.14 @mui/utils: 5.15.14 @mui/x-charts: ^7.2.0 => 7.3.0 @mui/x-data-grid: 7.3.0 @mui/x-data-grid-premium: ^7.2.0 => 7.3.0 @mui/x-data-grid-pro: 7.3.0 @mui/x-date-pickers: ^7.2.0 => 7.2.0 @mui/x-license: 7.2.0 @types/react: 18.2.61 => 18.2.61 react: 18.2.0 => 18.2.0 react-dom: 18.2.0 => 18.2.0 typescript: 5.3.3 => 5.3.3 ``` Chrome

Search keywords: grouping flex pinned Maximum update depth exceeded Order ID: 70572

beaktor commented 2 weeks ago

Thanks for setting it up @cstephens-cni. I see the same error in my test environment, even when stripped down to a flat (non-grouping) table, 1 column, 1 row.

(I'm not able to fork your example to create the absolute simplest example, but hopefully my description should suffice.)

As a side note, this is gating our adoption of 7.x, as we're stuck on the latest 6.x until this is resolved. Thanks!

michelengelen commented 1 week ago

Hey @cstephens-cni and @beaktor ... I can confirm this bug. On a side note: This is not happening when on initial load the grid is already in the "sticky-state" (the rows are overflowing the container. There is a different problem with that as well: The width is not calculated correctly. cc @romgrk

romgrk commented 2 days ago

I can't reproduce with the codesandbox link above, page loads without error. Are you still observing the issue? If so, can you give more details?

beaktor commented 2 days ago

@romgrk I just clicked on the very first link above (live example) into an incognito window, and see the error:

image

Arguably, it's not the simplest repro scenario, with unnecessary multiple columns and column grouping. If you think it would help for an even simpler repro, I can try afresh. I was able to see column size problems with just the combination of 1) pinned column, and 2) flex: turned on for that pinned column.

romgrk commented 2 days ago
It's working for me. ![image](https://github.com/mui/mui-x/assets/1423607/25d97fa8-c495-4704-9558-216b5b38bc68)

Maybe it's a macOS issue, @MBilalShafi or @cherniavskii would one of you be able to reproduce? If so, could you take the issue?

beaktor commented 2 days ago

@romgrk I think the other key that can trigger it, is resizing the window in that configuration. Would you mind trying that? Thank you!

beaktor commented 1 day ago

@romgrk Any update if you were able to reproduce this, if you resized the window? If not, I'll try and create another sandbox from scratch to see if it's simpler.