[data grid] Grid crashes when switching from tree data to regular grid

[Zitrooone]

Steps to reproduce

Steps:

1. Open this link to live example: Link
2. Select "Job title" as grouping in the select at the top
3. Switch back to "None"
4. Cannot read properties of undefined (reading 'depth')

Current behavior

The grid crashes when I "turn off" the treeData view.

Expected behavior

The grid does not crash and treeData is turned off, which results in a regular grid being displayed.

Context

This was working just fine with V6 of the DataGrid. I recently wanted to migrate to V7 and started encountering this issue. Before I crafted the example I encountered another error, which was Could not find row with ID #0. I can no longer replicate it though.

I tried to build an example that is as close as possible to what we are using. For some reason codesandbox has issues previewing the example Rendered fewer hooks than expected. This may be caused by an accidental early return statement.. Is this a problem with my setup or with codesandbox?

Your environment

npx @mui/envinfo

System:
    OS: Windows 11 10.0.22631
    @mui/private-theming:  6.1.9
    @mui/styled-engine:  6.1.9
    @mui/system:  6.1.9
    @mui/types:  7.2.19
    @mui/utils:  6.1.9
    @mui/x-data-grid:  7.23.0
    @mui/x-data-grid-pro: ^7.23.0 => 7.23.0
    @mui/x-date-pickers: ^7.23.0 => 7.23.0
    @mui/x-internals:  7.23.0
    @mui/x-license: ^7.23.0 => 7.23.0
    @types/react: ^18.3.12 => 18.3.12
    react: ^18.3.1 => 18.3.1
    react-dom: ^18.3.1 => 18.3.1
    typescript: ^5.7.2 => 5.7.2

Search keywords: treeData
Order ID: 102617

[arminmeh]

Hey @Zitrooone thanks for reporting this.
I will look into it.

Regarding the Codesandbox, it is a known issue and we have opened an issue upstream.
Workaround has been merged and the issue should be gone with the next release.

More info here #15765

[arminmeh]

Is there a specific reason you are processing the rows the way you are in your example?

I found that this is the cause of the error
You can check updated example which uses the rows data (with hierarchy) directly and it works fine.

*I am also using the version of the grid that is not released yet but has the codesandbox issue resolved

[Zitrooone]

Hi @arminmeh, thanks for the heads up regarding the Codesandbox. And thank you for your example.

The reason we process the rows this way is that we have multiple groupings you could chose, which would dictate the way the hierarchy is built up. Essentially more cases in the getRows switch. Some groupings also have more depth than others. The original data is retained to be able to go back to a blank state, although I see that the hierarchy is ignored when not using treeData.

I was playing around with your suggestion, but kept stumbling across bits that make the current use of useMemo convenient.
We deploy many grids, all with different data structures and dependencies for the useMemo. Having a mostly unified way of keeping the rows in sync is very important. I will try to adapt your example to further showcase the situation.
While it's certainly possible, I would prefer to not rebuild all of those grids.

Any idea what causes the error in the first place?

[Zitrooone]

I tinkered with the example and made this, hoping it helps and does not cause more confusion. I will try to explain:
I've added two more columns and two more groupings, as well as a checkbox.
The checkbox will take effect when the grouping is set to "hobby". It will add all hobbies to the grid, even if no one is executing them. When unticking the checkbox I need to remove those rows, which can be done by either using the original data or filtering the current data with the key isHobbyOnly.
So now my dependencies are the grouping and the checkbox. I can add those to the useMemo along with any additional dependencies that might be added in the future. The other route I can think of would be to trigger a row update in every change handler of any of the dependencies. The latter would be easier to make mistakes with and less of an obvious pattern than the former, at least to me.

I hope this made sense. If not, I apologise! I will look into this topic further tomorrow with a fresh head.

PS: Knowing the cause of the error might help me figure out what to avoid, if it is indeed a fault in my architecture. I was considering it being some sort of out-of-sync data due to the useMemo, but I cannot make sense of it unfortunately.

[arminmeh]

Thanks for all the info and additional example @Zitrooone

This is what I have observed last time

• State is updated multiple times by changing the group value (without useMemo only once)
• In one of those updates, there is a mismatch of the state parts which causes the error (accessing nonexistent record)
• If a guarding condition is added to ignore this mismatch grid eventually ends up in the correct state.

I have to investigate why the mismatch happens. If it is on our side I can fix that, but you should still be aware that this setup re-renders grid multiple times.

Once I get more details I will get back to you.

[Zitrooone]

Thanks for the feedback @arminmeh

Looking at the code I would think that isTree might be the bit that is not matching the data, but wrapping that in a useMemo does not appear to fix the issue either. Maybe it is time to give that sweet new compiler a chance 😄
Jokes aside, I will await your feedback. Thank you for your time.

[arminmeh]

Hi @Zitrooone

I have found the root cause of the error and found a way for your last example to work with the current data grid.

If you look at first warnings in Row definition and Column definition pages you can see that it is not expected to deal with rows and columns in the way you did.

But I think that the grid should still handle your case, only with the mentioned performance degradation.

Error is caused by the way the updates are being processed when both treeData and rows reference is updated.
I will discuss with the team how to adjust this, but it might take a bit longer until this is updated.

This is why I have updated the sandbox to keep the rows reference stable (when treeData is updated) and allow you to move forward.

Let me know if something else (not included in your last example) is blocking you to keep the reference stable.

[Zitrooone]

Hi @arminmeh

Thank you for your update. I don't quite understand why the getRows was moved to be a useCallback. What I did notice when looking at your example is that you did not use structuredClone and instead modified gridData directly. I tried this in our code and it worked without further modifying the architecture. I see how this stabelises the reference for the rows, but in case we add the hobbies to the list, the reference would be different again? Surprisingly this does not cause issues.

Unfortunately my time to test is very limited, as I'm leaving for vacation very soon. I can do a deep dive and implement this fix in our various grids early next year. Let me know if you need anything from me.

[arminmeh]

I don't quite understand why the getRows was moved to be a useCallback.

You can ignore this, I should have cleaned it up.
While I was doing some debugging I was moving things around. I have updated sandbox again to be aligned with your example.

I see how this stabelises the reference for the rows, but in case we add the hobbies to the list, the reference would be different again? Surprisingly this does not cause issues.

Issue only happens if treeData and rows are updated at the same time and if treeData is changing from true to false.
This is why it only happened while you were reverting to 'None'.
When you add all Hobbies, you are changing rows, but not treeData.

[arminmeh]

Note for the team

The error happens because when both treeData and rows are updated, grid starts re-render and strategy processors availability is calculated again.

Problem is, for rowTreeCreation grid is not aware immediately that the strategy has changed.
It detects that the processor has changed for the tree-data strategy here
This triggers row and lookup recalculation, but with the new rows reference it clears rows cache

So, in this cycle it tries to build tree data lookups, but without any rows which crashes the grid.
If the rows reference is not updated, cache is used, which completes this cycle after which grid detects strategy change to default and does row creation again.

Fixing this problem will also get rid of this extra cycle which will help performance.

[cherniavskii]

Another workaround for this issue is to change the key prop to reset the data grid state when switching to tree data and back: https://codesandbox.io/p/sandbox/datagrid-treedata-crash-forked-g6kdm4?file=%2Fsrc%2FDemo.tsx%3A135%2C1

[Zitrooone]

Hi @arminmeh
I now had some time to work on the conversion of our grids and managed to have the first grid run error free. That grid was the base for the example I crafted.
Onto the next one, I was not able to resolve the issue in the same way unfortunately. Instead I tried porting the logic to using the API, which I understood to be the desired and more performant way. Calling updateRows to set the hierarchy according to the selected grouping now crashes with Cannot read properties of undefined (reading 'id'). I've created an updated example which recreates the issue.
Some of my findings (also mentioned in comments in the example):

• Using any initial value other than GroupsEnum.NONE for group causes the error
• In my code I was able to prevent this error by wrapping the updateRows in setTimeout, which oddly does not work in the example
• Using setRows instead of updateRows works reliably, but (in my eyes) defeats the purpose of not changing the rows prop itself

This leads me to believe, that preventing the initial hierarchy update by returning the data from the server with the proper hierarchy set would also prevent any issues. However, this would mean replicating the hierarchy logic in the backend (which also uses another language), which seems rather error prone to me.

Note: The grid I am currently working on is more complex than the previous one and allows users to create, update and delete rows. This has been ported to using updateRows as well and seems to be working fine. Since it has not caused any issues (yet - I have not tested it much), it was omitted from the example.

Any guidance on how to handle row changes the intended way would be greatly appreciated.

[arminmeh]

@Zitrooone this is happening because the tree starts to build up before the hierarchy is being added to the data.
I have tweaked your example to have isTree as another state const [isTree, setIsTree] = useState(false);

This state is then updated in updateHierarchy to have the changes in the same cycle

function updateHierarchy(groupValue: GroupsEnum) {
    if (!gridApi.current) {
      return;
    }

    setIsTree(groupValue !== GroupsEnum.NONE);

    // Rows can be added, deleted, or updated using the grid API, so we need to get the current rows
    const currentRows = gridApi.current.getRowModels();
   ...

After this, it seems that everything is working fine

[Zitrooone]

@arminmeh thank you very much!
I was able to resolve all my issues.