[data grid] valueFormatter doesn't show the updated value after processRowUpdate

[atsoy]

[data grid] valueFormatter doesn't show the updated value after processRowUpdate

[michelengelen]

@DanailH gentle ping! 🙇🏼

[cherniavskii]

This looks like a regression introduced in 6.18.3
v6.18.2: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-4skj6q
v6.18.3: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-mqfp7x

[DanailH]

@atsoy thanks for reporting this. The issue is with the return value of the 'handleProcessRowUpdate' - it should be an array with the mutated row objects. You can see it working here: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-2slznj?file=%2Fsrc%2Fdemo.tsx%3A110%2C32

[atsoy]

Thanks @DanailH and HNY.
In the provided example Typescript complains about types for processRowUpdate, since it expects just a R.

Was already surprised, cause we've used to return a single row, not an array.

[DanailH]

@atsoy I check it again, there indeed might be a problem with the valueFormatter in combination with processRowUpdate. I'll investigate a bit more and see what I can find.

[DanailH]

Why do you need to keep your rows in the state? This seems to be causing the problem. if you check this example the issue will be more visible: https://mui.com/x/react-data-grid/editing/#server-side-persistence

[atsoy]

Thanks for checking @DanailH !

The example I've provided based on local state, in reality we use reducer (useReducer), where our data is being stored. Users can manipulate table data and then send it to the server (not after each edit stop, but form like - pressing on submit button).

I'm not sure I understand the view point of "storing data in state seems to be causing the issue". Updated data is being set properly. Please elaborate.

P.S: The process of updating the rows, formatting updated values etc worked without issues by using processRowUpdate and valueFormatter until we've upgraded the version to ~6.18.3 (previous version was ~6.12.1) Today tested with 6.18.7

[atsoy]

Hi @michelengelen @DanailH , I understand that you're working on this project voluntarily, and I truly appreciate the time and effort you invest in it. If possible, could you provide an ETA for when this issue might be fixed?

Thank you once again for your dedication to the project!

[DanailH]

@atsoy I'm sorry for the delay. I checked it again and the issue is that handleProcessRowUpdate expects a return value - the updated row. As far as I can tell you are updating your state that contains the rows and then expect the grid to rerender with the new values which doesn't happen. You need to return the updated value of the row you are editing with the handleProcessRowUpdate.

I've updated the example again https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-9kxt24

[atsoy]

@DanailH thanks for coming back!
I actually do not really understand, why should I manipulate the newRow to display updated product of 2 fields. I mean in the example I've provided - it was simplified by using useState, in reality we use useReducer.

I've updated provided example.

As far as I can tell you are updating your state that contains the rows and then expect the grid to rerender with the new values which doesn't happen.

It actually happens, but after that, I assume grid re-renders after some effect and restores probably from internal grid state.

[DanailH]

@mui/xgrid can someone take a look? It might be that I don't understand the issue.

[cherniavskii]

While this looked like a regression at first glance, I'm not sure about this now.
I've created a simplified demo with a delay after updating the rows' state.
With the delay, the issue is reproducible in both 6.18.2 and 6.18.3:
v6.18.2: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-qf8769
v6.18.3: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-vf2zn7

Added delay makes it clear that it's a race condition - the rows state gets updated with a new total field value, but the newRow returned from processRowUpdate has the old total value.
Before 6.18.3, the rows state update takes precedence over newRow, but starting from 6.18.3 the newRow it's not the case anymore.

@atsoy Before we proceed with further investigation, did you consider using valueGetter to derive the total column? Here's a demo: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-wy8qgp

[atsoy]

@cherniavskii thanks for your input!

Before we proceed with further investigation, did you consider using valueGetter to derive the total column? Here's a demo: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-wy8qgp

Yes, I'll try to use that as a workaround and will give an update

P.S: I'm not sure if it's the right place, but did you or do you consider to provide e.g an option / method to access the current grid data as it is (since there are already possibilities to export CSV or printable data).

[cherniavskii]

do you consider to provide e.g an option / method to access the current grid data as it is

The same data that you pass to the grid through the rows prop? What is the use case for this?

[atsoy]

Sorry, I mean of course not the same data, which is provided to rows, but the current State of it (e.g after some data manipulations).

Use case e.g to get avoid usage of useState and useReducer and get the data direct out of grid (let's say for submitting a form with some other fields and grid data).

[atsoy]

Hey @cherniavskii , thanks again for the provided workaround idea. I've implemented it in multiple tables. However I think I found another bug, which is be related to the existing one (or may be have same source):

In case of the let's say price field i expect the input like 2,33 or 2.33, which will be validated, parsed and stored as 2.33 in the reducer state. In case user types 2,33 (with comma), it renders NaN in the end, but should actually show 2.33 (with dot).

I didn't add the preProcessEditProps validation, but we have one, which checks the input and shows error if something is wrong (as a tooltip).

See example

If you uncomment (valueGetter) it will work fine, but still not the desired functionality (imo), because of provided data, which should be updated in the internal grid state as well and rendered in cells.
Or I might understand the grid component design and purpose wrong..

[atsoy]

to follow up: will this be considered as a bug or may be analized? I ask because of this statement.

Before 6.18.3, the rows state update takes precedence over newRow, but starting from 6.18.3 the newRow it's not the case anymore.

[cherniavskii]

Hey @atsoy
Sorry for the late reply!

I've looked into the codesandbox you provided.
First of all, the reason you see NaN after editing is that you use the string column type (it's a default) for numeric data, this is why the string is not parsed into the number properly. Adding type: "number" solves the issue.
Then, I uncommented the valueGetter for the total column since you want to derive the value for this column from other fields of the row.
Here's a working demo: https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-forked-93cfzv?file=%2Fsrc%2Fdemo.tsx
Note, that the row data passed to processRowUpdate is now properly parsed.
Is this the desired result?

[cherniavskii]

I mean of course not the same data, which is provided to rows, but the current State of it (e.g after some data manipulations).

Not sure if you're still going to need it, but you can get a single row using API object - apiRef.current.getRow(id).
To get all row data, you can use gridRowsLookupSelector(apiRef) to get the rows lookup and then convert it to an array.
Hope this helps!