Closed atsoy closed 7 months ago
@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
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.
@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.
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
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
@DanailH gentle ping! 🙇🏼
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!
@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
@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.
@mui/xgrid can someone take a look? It might be that I don't understand the issue.
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
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
@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).
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?
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).
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..
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.
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?
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!
The issue has been inactive for 7 days and has been automatically closed.
Hi @cherniavskii thanks for explanation. I think this cannot be considered as a bug. So I just comment it and will close this issue.
Again, thanks for the efforts!
:warning: This issue has been closed. If you have a similar problem, please open a new issue and provide details about your specific problem. If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.
How did we do @atsoy? Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief survey.
Steps to reproduce
Link to live example: (required): https://codesandbox.io/p/sandbox/dgp-cellupdate-bug-rzkwcr
Steps:
Current behavior
total
column does not display updated value (product ofamount
andprice
).As soon as another row's cell get an updated value, then previous edited row's
total
shows correct valueExpected behavior
total
column shows updated value, after one of the columnsamount
||price
were updated.Context
Idea ist to update the total column's cell in the same row, which multiplies
amount
andprice
columns, after cell editing ist finished.After some analysis I've noticed, that
valueFormatter
actually does get the updated value and formats it correct, but after that it's being called again, but with old value.P.S: It worked before, but since we've updated
x-data-grid-pro
to the 6.18.3 it was broken (also tested with6.18.5
)Your environment
``` System: OS: macOS 13.6.2 Binaries: Node: 18.13.0 Yarn: 1.22.19 npm: 8.19.3 Browsers: Chrome: 119.0.6045.159 Safari: 17.1.2 npmPackages: @emotion/react: ~11.11.1 => 11.11.1 @emotion/styled: ~11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.27 @mui/core-downloads-tracker: 5.14.7 @mui/material: ~5.14.7 => 5.14.7 @mui/private-theming: 5.14.7 @mui/styled-engine: 5.14.7 @mui/system: 5.14.7 @mui/types: 7.2.4 @mui/utils: 5.15.0 @mui/x-data-grid: 6.18.5 @mui/x-data-grid-pro: ~6.18.3 => 6.18.5 @mui/x-date-pickers: 6.18.5 @mui/x-date-pickers-pro: ~6.18.3 => 6.18.5 @mui/x-license-pro: 6.10.2 @types/react: 18.2.24 => 18.2.24 react: ~18.2.0 => 18.2.0 react-dom: ~18.2.0 => 18.2.0 typescript: 5.2.2 => 5.2.2 ```npx @mui/envinfo
Search keywords: datagridpro processrowupdate valueformatter Order ID: 62555