[DataGrid] Data source with editing #16045

MBilalShafi · 2024-12-31T11:44:43Z

Resolves #13261

Preview

In progress items:

Implement with cell editing
Implement with row editing
Support editing in useMockServer
Update the plan for the base data source and incorporate the changes ([DataGrid] Make base data source available in the Community plan #16359)
Improve/unify error handling mechanism
Documentation improvements
Improve cache mutation
🚧 Testing

Future improvements:

Performance: Move computation of rowIdToGetRowsParams to useTransition or similar delayed computation mechanism (Should we ?)

mui-bot · 2024-12-31T11:50:21Z

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

Updated pages:

Generated by 🚫 dangerJS against d5e8795

MBilalShafi · 2025-02-17T14:32:08Z

packages/x-data-grid/src/tests/dataSource.DataGrid.test.tsx

@@ -160,8 +160,8 @@ describeSkipIf(isJSDOM)('<DataGrid /> - Data source', () => {

    setProps({
      paginationModel: { page: 1, pageSize: 10 },
-      sortModel: [{ field: 'name', sort: 'asc' }],


Adding the warning surfaced this hidden issue in the test, it was trying to set the sortModel field to name which was not there.

MBilalShafi · 2025-02-17T14:46:33Z

packages/x-data-grid/src/models/props/DataGridProps.ts

   */
-  unstable_onDataSourceError?: (error: Error, params: GridGetRowsParams) => void;
+  unstable_onDataSourceError?: (error: GridDataSourceError) => void;


Breaking change

It's updated for a better DX for errors originating from different operations (fetch, update, etc.).

GridDataSourceError is a custom error class extended from the Error class, it adds params object and appropriate methods to support different contexts (params) for different originating sources (currently we have getRows and updateRow, but this could be extended to let's say deleteRow, addRow, etc.)

The alternate would be to define different set of props for each use case:
getRows: onDataSourceGetRowsError
updateRows: onDataSourceUpdateRowsError

I personally like handling it at one place to have less API surface. Open to comments.

I am also for having one prop to handle all errors. Errors can have different classes that extend some base class to keep the common stuff together and have specific where needed.

cherniavskii · 2025-02-17T15:06:01Z

packages/x-data-grid/src/hooks/features/dataSource/gridDataSourceError.ts

+  /**
+   * Returns true if this error was caused by a fetch operation
+   */
+  isFetch(): this is GridDataSourceError & { params: T } {


Having methods on the error object is a bit unusual.
Did you consider something like this instead?

type DataSourceErrorParams = | [{ type: 'fetchRows', message: string }, { page: number }] | [{ type: 'updateRow', message: string }, { rowId: string }]; interface Props { unstable_onDataSourceError: <T extends DataSourceErrorParams>( ...args: T ) => void; }

Thank you for the suggestion, I did actually try a similar approach, it does have its pros but I decided to not use it for a few reasons, which I'll state below.

For a code-comparison I prepared a drilled-down version here.

Type safety: As you can notice, doing if (error.isFetch()) automatically makes the Typescript infer the params type and no manual type assertion is needed.

Readability: In my personal opinion error.isFetch() is more readable/self-explanatory than error.type === 'fetchRows'

Extensibility: Adding more methods won't require updating the typescript function signature.

Better Debuggable (opinionated): Due to methods like instanceof available on Error and it's derived classes, one can differentiate it with other errors by doing error instanceof GridDataSourceError, which might improve debugging experience.

On a second thought, I think it makes more sense to introduce different error signatures for each error and keep relevant parameters in the error class in order to:

Nice DX. Use the instanceof to determine the type of error, error instanceof DataSourceFetchRowsError and params type is inferred automatically. (It also seems to be a widely used pattern for similar use-cases)

Keep the signature of the handler consistent.

Open the possibility of future extensibility.

Something like this.

Wdyt?

I am also not sure if I used an error object with these kind of methods before.
Differentiate them by class looks better. I would also go for both
Have different classes and have method prop in the error so that devs can use whatever they like more.

Method prop has a slight disadvantage of having to manually do type assertions, differentiating by class (second link in my above comment) looks most appropriate to me

packages/x-data-grid-generator/src/hooks/useMockServer.ts

arminmeh · 2025-02-17T18:36:36Z

packages/x-data-grid/src/models/props/DataGridProps.ts

   */
-  unstable_onDataSourceError?: (error: Error, params: GridGetRowsParams) => void;
+  unstable_onDataSourceError?: (error: GridDataSourceError) => void;


I am also for having one prop to handle all errors. Errors can have different classes that extend some base class to keep the common stuff together and have specific where needed.

arminmeh · 2025-02-17T18:43:39Z

packages/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts

+                message: errorThrown?.message,
+                operationType: 'updateRow',
+                params: updateParams,
+                cause: errorThrown,


maybe context?

context should also be fine, I just wanted to be more aligned with the Error.cause since it's description seemed to match our use-case.

It is used when catching and re-throwing an error with a more-specific or useful error message in order to still have access to the original error.

Wdyt?

arminmeh · 2025-02-17T18:43:52Z

packages/x-data-grid/src/hooks/features/editing/useGridRowEditing.ts

+            props.unstable_onDataSourceError(
+              new GridDataSourceError({
+                message: errorThrown?.message,
+                operationType: 'updateRow',


Suggested change

operationType: 'updateRow',

method: 'updateRow',

arminmeh · 2025-02-17T18:47:37Z

packages/x-data-grid/src/hooks/features/dataSource/gridDataSourceError.ts

+  /**
+   * Returns true if this error was caused by a fetch operation
+   */
+  isFetch(): this is GridDataSourceError & { params: T } {


I am also not sure if I used an error object with these kind of methods before.
Differentiate them by class looks better. I would also go for both
Have different classes and have method prop in the error so that devs can use whatever they like more.

arminmeh · 2025-02-17T18:51:53Z

packages/x-data-grid/src/hooks/features/dataSource/useGridDataSourceBase.ts

+        return;
+      }
+      updatedRows[rowIndex] = rowUpdate;
+      cache.set(getRowsParams, { ...cachedData, rows: updatedRows });


It is way more complicated than this 😬

This only works if sorting and/or filtering is not applied. Otherwise, you need to have this row in some other place or not in this cache key at all anymore. Not only that, but it affects other cache keys as well.

I am not event sure if we can do anything to keep the cache once sort/filter is there unless we don't change the current cache structure completely.

A composite key may be?

Instead of 'row.id' only, we create a key for all the differentiating factors e.g.

const key = JSON.stringify([row.id, filterModel, sortModel, groupKeys])

const value = getRowParams

Then, when searching for a specific row, we generate that composite key again based on the current model values.

I'll give it a try.

The problem is the position
If you sort by certain column and you change the value of that column, you need to update all chunks in between and move one record up and then inject updated record in the chunk which holds the position that the new value occupies.

Depending on how complex you want to go I see two possibilities with the current cache structure.

You only keep cache chunks with the pagination param only (since it doesn't affect position or other data) and clear all others

You track which columns have changed and you update all cache chunks that have sort/filter/group/aggregation params that are not related to the updated columns and clear the rest

As mentioned above, updating cache for the columns that are updated is pretty hard. Problem mentioned for sorting applies for other operations that we cache - you need new aggregation amount or you need to remove the row completely for some filter param because it becomes filtered out (which means you have to adjust some other chunks related to that filter param), etc.

You track which columns have changed and you update all cache chunks that have sort/filter/group/aggregation params that are not related to the updated columns and clear the rest

@arminmeh You made some great points. I agree that delving deeper into this approach could be highly complex and performance-intensive. More importantly, Data Grid cache should not handle business logic owned by the server, such as server-side sorting, especially since it also lacks access to all the data.

If we extract the models which might affect the visibility or positioning of rows when an edit is made, they would be:

Model Reasoning

sortModel Determines row positions; an edit might cause a row to move, requiring a refetch to maintain correct order. The edited row could literally be anywhere, even in the un-fetched chunks.

filterModel Controls row visibility; an edited row may no longer match the filter criteria and needs to be removed or added dynamically.

aggregationModel Affects aggregated values; editing a row could change totals, averages, or other computed metrics, necessitating a refresh.

If any of these models are active, the Data Grid cannot accurately determine the edited row’s position without access to server-side business logic and data, which it does not have. Additionally, it cannot predict which row, if any, should replace a removed one in the current chunk.

The best option I see here is a hybrid approach where we partially reap the benefits of caching with editing.

✅ On every edit operation, if no sorting, filtering, or aggregation is active → Mutate cache entry
The users fully enjoy the client-side caching benefits in this case

❌ If any of those models are active → Invalidate cache and refetch the current viewport
This case would be one of the limitations of the client-side cache, i.e. as soon as an edit is made, the cache has to be cleared.

💡 As an alternative, we could urge the users to consider server-side caching in cases where editing has to be used with one or more of sorting, filtering, and aggregation models, since the server has access to all the data and could provide a more reliable caching.

Lmk how you feel about this approach?

arminmeh · 2025-02-17T18:55:11Z

packages/x-data-grid/src/hooks/features/dataSource/useGridDataSourceBase.ts

@@ -125,6 +132,9 @@ export const useGridDataSourceBase = <Api extends GridPrivateApiCommunity>(
        const cacheResponses = cacheChunkManager.splitResponse(fetchParams, getRowsResponse);
        cacheResponses.forEach((response, key) => {
          cache.set(key, response);
+          response.rows.forEach((row) => {
+            rowIdToGetRowsParams.current[row.id] = fetchParams;


one row will be present in multiple cache keys when sorting/filtering/grouping is applied

Co-authored-by: Armin Mehinovic <[email protected]> Signed-off-by: Bilal Shafi <[email protected]>

[DataGrid] Data source with editing

b030773

MBilalShafi added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request feature: Server integration Better integration with backends, e.g. data source labels Dec 31, 2024

Merge branch 'master' into datasource-with-editing

ddf5c50

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 9, 2025

This comment was marked as outdated.

Sign in to view

Merge branch 'master' into datasource-with-editing

06f994a

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 13, 2025

Merge branch 'master' into datasource-with-editing

2cb87d8

This comment was marked as outdated.

Sign in to view

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 24, 2025

Merge master into 'datasource-with-editing'

ec56d8f

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Jan 27, 2025

This comment was marked as outdated.

Sign in to view

Merge branch 'master' into datasource-with-editing

531644c

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 10, 2025

MBilalShafi added 11 commits February 11, 2025 16:28

Add to row editing

830c7c4

Merge branch 'master' into datasource-with-editing

4852d63

Merge branch 'master' into datasource-with-editing

9ec15f4

Update the error handling mechanism

82b96d6

Add the possibility to end the edit mode in case of error

a8625c9

Add development warnings

473bec3

Docs update

e15f278

Add parenthesis and use gridRowIdSelector

6b7bbac

Update the variables' order

7479c61

Rebuild docs

8e1493c

Fix incorrect auto-import

ef14f00

MBilalShafi added 4 commits February 17, 2025 14:08

Merge branch 'master' into datasource-with-editing

f77be1b

Fix the test throwing error

4aeddeb

docs:api

9388222

Merge branch 'master' into datasource-with-editing

6d76951

MBilalShafi marked this pull request as ready for review February 17, 2025 14:09

MBilalShafi commented Feb 17, 2025

View reviewed changes

Merge branch 'master' into datasource-with-editing

75fead2

cherniavskii reviewed Feb 17, 2025

View reviewed changes

arminmeh reviewed Feb 17, 2025

View reviewed changes

MBilalShafi marked this pull request as draft February 19, 2025 16:28

MBilalShafi and others added 3 commits February 20, 2025 16:35

Update error message

b40ec49

Co-authored-by: Armin Mehinovic <[email protected]> Signed-off-by: Bilal Shafi <[email protected]>

Use different error objects for getRows and updateRow

5d33203

Merge branch 'master' into datasource-with-editing

d5e8795

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Data source with editing #16045

[DataGrid] Data source with editing #16045

MBilalShafi commented Dec 31, 2024 •

edited

Loading

mui-bot commented Dec 31, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

MBilalShafi Feb 17, 2025 •

edited

Loading

MBilalShafi Feb 17, 2025

arminmeh Feb 17, 2025

cherniavskii Feb 17, 2025

MBilalShafi Feb 17, 2025 •

edited

Loading

arminmeh Feb 17, 2025

MBilalShafi Feb 17, 2025 •

edited

Loading

arminmeh Feb 17, 2025

arminmeh Feb 17, 2025

MBilalShafi Feb 20, 2025

arminmeh Feb 17, 2025

arminmeh Feb 17, 2025

arminmeh Feb 17, 2025

MBilalShafi Feb 17, 2025

arminmeh Feb 17, 2025

arminmeh Feb 18, 2025 •

edited

Loading

MBilalShafi Feb 20, 2025 •

edited

Loading

arminmeh Feb 17, 2025 •

edited

Loading

Model	Reasoning
`sortModel`	Determines row positions; an edit might cause a row to move, requiring a refetch to maintain correct order. The edited row could literally be anywhere, even in the un-fetched chunks.
`filterModel`	Controls row visibility; an edited row may no longer match the filter criteria and needs to be removed or added dynamically.
`aggregationModel`	Affects aggregated values; editing a row could change totals, averages, or other computed metrics, necessitating a refresh.

[DataGrid] Data source with editing #16045

Are you sure you want to change the base?

[DataGrid] Data source with editing #16045

Conversation

MBilalShafi commented Dec 31, 2024 • edited Loading

mui-bot commented Dec 31, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

MBilalShafi Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Breaking change

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MBilalShafi Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MBilalShafi Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arminmeh Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

MBilalShafi Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

arminmeh Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

MBilalShafi commented Dec 31, 2024 •

edited

Loading

mui-bot commented Dec 31, 2024 •

edited

Loading

MBilalShafi Feb 17, 2025 •

edited

Loading

MBilalShafi Feb 17, 2025 •

edited

Loading

MBilalShafi Feb 17, 2025 •

edited

Loading

arminmeh Feb 18, 2025 •

edited

Loading

MBilalShafi Feb 20, 2025 •

edited

Loading

arminmeh Feb 17, 2025 •

edited

Loading