Skip to content

Conversation

@sai6855
Copy link
Contributor

@sai6855 sai6855 commented Oct 10, 2025

closes #19370

Before

https://stackblitz.com/edit/github-p76sjn2j?file=src%2Fdemo.tsx

After

https://stackblitz.com/edit/7ejhgqyd?file=src%2FDemo.tsx

Root Cause

When data loads asynchronously:

  1. Grid initializes with 0 rows → aggregation calculates empty lookup {} and marks as applied
  2. Rows finish loading → rowsSet fires → triggers aggregation
  3. Sorting reapplies → sortedRowsSet fires immediately after → triggers aggregation with reason: 'sort'
  4. Due to useRunOncePerLoop batching, only ONE runs - the 'sort' one
  5. Optimization logic skips: reason === 'sort' && !needsSorting && hasApplied → returns early
  6. Result: aggregation stays empty forever

Solution

  • Track if aggregation has been applied at least once (hasAppliedAggregationRef)
  • On rowsSet event, reset the flag to false
  • This ensures aggregation must recalculate when rows load, even if a sort event immediately follows

Why the optimization is still needed

The optimization prevents unnecessary recalculations when sorting columns that don't have applySorting: true (like
sum, avg, etc.). The fix preserves this while ensuring we never skip the initial calculation after data loads.

@sai6855 sai6855 marked this pull request as draft October 10, 2025 12:12
@sai6855 sai6855 added type: bug It doesn't behave as expected. docs Improvements or additions to the documentation. scope: data grid Changes related to the data grid. plan: Premium Impact at least one Premium user. and removed docs Improvements or additions to the documentation. labels Oct 10, 2025
@mui-bot
Copy link

mui-bot commented Oct 10, 2025

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

Bundle size report

Bundle Parsed size Gzip size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium 🔺+92B(+0.01%) 🔺+51B(+0.03%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against a994508

@lauri865
Copy link
Contributor

I have an alternative fix here: #19892

The issue is that it cancels too early if it's due to sorting. But there's another performance improvement I introduced there.

@sai6855 sai6855 changed the title [data grid] Fix column aggregation breaking if sortModel is passed to props [data grid] Fix column aggregation breaking Oct 10, 2025
@sai6855 sai6855 requested review from MBilalShafi, arminmeh and cherniavskii and removed request for MBilalShafi October 10, 2025 13:56
@sai6855 sai6855 marked this pull request as ready for review October 10, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plan: Premium Impact at least one Premium user. scope: data grid Changes related to the data grid. type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[data grid] Column aggregation breaks if sortModel is passed to props

3 participants