Material-table: Material-Table Mutates Item Passed Through data Prop

[kbi-daniel]

Describe the bug
We are passing an array of objects from our app's Redux state to the data prop. Material Table mutates that Redux state by adding a tableData property to each object in the array. This unexpected mutation of our apps Redux state caused some of our data to be corrupted when we saved the state back to our database (now with the inclusion of the tableData property.

To Reproduce
Steps to reproduce the behavior:

1. Pass an array of objects into the data prop.
2. console.log the array of objects on component render
3. note that each object in array now has the added tableData property.

Expected behavior
Material Table should not mutate items passed to data prop.

[kbi-daniel]

I created fork that solves the mutation problem. However, I am not sure if this approach has any other unforeseen consequences.
https://github.com/kbi-daniel/material-table/tree/bug/prevent-data-prop-mutate

[Domino987]

This already known and was tried to be solved #1174 here but it was reverted.
Because this removes the original reference to the existing data item on callbacks.
If the object is searched via shallow compares like this:

onRowClick={(event, clickedRow)=> {
    const existingRow= data.find(d => d === clickedRow) // Will always return undefined because reference changed
}

it will fail because the reference changed.

The way material-table saves its data has to be changed to turn the dependencies around like
row = {id: index, rowData: row }};
And what is written in tableData is just saved in the row object while the original data stays the same as the rowData object (same reference and no mutation).
But since accessing the data would have to be changed at all places, this will be a lot of work since tableData has 200 references in the project which all have to be changed to fit the new scheme.

I am a strong advocate to remove this prop mutation but as I said, it will be a lot of work.

Maybe be can create a branch and work on it together?
@mbrn , what do you think?

[amickael]

I've had the same issue, it becomes especially bad when dealing with Redux. Hopefully it will be addressed in the future (I'd be happy to help), but right now I've worked around it by passing data as such:

<MaterialTable data={tableData.map(item => Object.assign({}, item))} ... />

That way the data passed to the component is a copy instead of the original, where tableData is being supplied by state/prop/selector so it can update with changes.

[kbi-daniel]

@amickael we came up with a similar solution as well.

That being said, does it make sense to anyone else (and do you think it would work), to add a small utility function to the package to basically duplicate the code suggested by @amickael.

For example...

import MaterialTable, {noMutate} from 'material-table'

<MaterialTable data={noMutate(reduxData)} ... />

[Domino987]

While this might work for most use cases, it does not fix the underlying problem and the reference would still be different if that is needed.
Additionally, it would remove the selection, filter etc on every render of the parent.

[amickael]

@Domino987 Agreed, this is not a permanent solution. It also will not work if you have deeply nested structures. I wouldn’t recommend this approach be implemented inside of the component.

[Domino987]

@amickael , @kbi-daniel Do you guys agree that row = {id: index, rowData: row, ...otherTableRelatedData }}; is the best way to do that and return rowData for the callbacks and access the field with row.rowData.field?

Or how would you represent the data?

[amickael]

@Domino987 I think that's fine if rowData is not mutated afterward.

[Domino987]

Ok I guess we could open a new branch and start to change all the 200 relevant sections.

[narciero]

It’s worth mentioning that anyone using @apollo/client v3 will be unable to use this library as the query results (which I display in a table) are deeply frozen and therefore an exception is thrown when you try to set tableData

[Domino987]

Since this becomes more and more a problem, I will branch it tomorrow and will start on the changes. I'll link it here and hope some of you will make some PRs to it to get it done as soon as possible.

[Domino987]

On the other hand, if @mbrn agrees that we should not mutate the data and devs should not rely on shallow object reference, we could keep #1174 and just mark is as breaking change it it would be solved. That would be the easiest and cleanest in my opinion. What do you say @mbrn ?

[cutamar]

@Domino987 any updates on this? I am trying to use relay with hooks, but it won't work if material-table keeps mutating the data object.

[Domino987]

@cutamar Sorry for the delay. I open a branch, but as discussed, it needs a lot of work.
https://github.com/Domino987/material-table/tree/mutation-removal

[manuFL]

still no news from the master branch?

[kaviyarasu1987]

is it closed?

[Domino987]

We are currently closing the issues and PRs. After that we will rewrite to use Context and function components. During this, we will also remove the mutations. So I will close this issue, because its a known issue and we will fix this asap.

[geohuz]

Any news for the mutation-removal?