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

[DataGridPro] Fix lazy loading error when existing rows are passed to replaceRows #10821

Merged
merged 2 commits into from Feb 27, 2024

Conversation

martijn-basesoft
Copy link
Contributor

It seems there a sorting issue in combination with the lazy loader. When unstable_replaceRows is called having existing rows in a different order, useGridRows gives an error because it can't find rows because they have been deleted before.

This behavior can be seen in the following demo. This is a fork of the lazy loading example from the documentation with rowLength adjusted to 10. To trigger the error simply change the order of the Name column.
https://codesandbox.io/s/nifty-pike-slzjmd?file=/src/Demo.tsx

This PR includes a suggested fix and test. Let me know if any change is needed and we're happy to update it.

@mui-bot
Copy link

mui-bot commented Oct 27, 2023

Deploy preview: https://deploy-preview-10821--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against bfd43ab

@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Oct 27, 2023
@michelengelen michelengelen changed the base branch from master to next November 6, 2023 14:18
@martijn-basesoft
Copy link
Contributor Author

Any news on this? It seems to me that this is quite a bug.

@michelengelen michelengelen added the bug 🐛 Something doesn't work label Jan 2, 2024
@romgrk
Copy link
Contributor

romgrk commented Jan 2, 2024

I'm not sure I understand the problem and I haven't been able to trigger an error with the linked demo. Can you clarify the problem and the steps to reproduce?

@martijn-basesoft
Copy link
Contributor Author

You should be able to trigger the error if you open the sandbox demo:

Then I see the error in the screen:

TypeError
Cannot read properties of undefined (reading 'type')
    at eval (https://slzjmd.csb.app/node_modules/
mui/x-data-grid/hooks/features/rows/useGridRows.js:280:74
    at Object.eval [as unstable_replaceRows] (https://slzjmd.csb.app/node_modules/
mui/x-data-grid/hooks/features/rows/useGridRows.js:280:42
Object.eval
/src/Demo.tsx:88:21
  85 | async (params: GridFetchRowsParams) => {
  86 |   const { slice, total } = await fetchRow(params);
  87 | 
> 88 |   apiRef.current.unstable_replaceRows(params.firstRowToRender, slice);
     |                 ^
  89 |   setRowCount(total);
  90 | },
  91 | [apiRef, fetchRow]

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@romgrk
Copy link
Contributor

romgrk commented Feb 14, 2024

I've been able to reproduce the issue. Could you solve the conflicts so we can merge the PR?

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 27, 2024
@kwek
Copy link

kwek commented Feb 27, 2024

@romgrk: I rebased the PR against mui-x/next branch. I didn't get any conflicts however. Is it ok to merge now?

@romgrk romgrk enabled auto-merge (squash) February 27, 2024 12:42
@romgrk romgrk merged commit 819f625 into mui:next Feb 27, 2024
15 checks passed
@cherniavskii cherniavskii changed the title [DataGridPro] Fix useGridRows not giving error on reversed data [DataGridPro] Fix lazy loading error when existing rows are passed to replaceRows Mar 1, 2024
@cherniavskii cherniavskii added plan: Pro Impact at least one Pro user feature: Row loading Related to the data grid Row loading features needs cherry-pick The PR should be cherry-picked to master after merge labels Mar 1, 2024
cherniavskii pushed a commit to cherniavskii/mui-x that referenced this pull request Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Row loading Related to the data grid Row loading features needs cherry-pick The PR should be cherry-picked to master after merge plan: Pro Impact at least one Pro user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants