Pagination change issue

[edgar-pozadas-by]

Here you have an example with the error
https://codesandbox.io/s/pagination-zu4gw?file=/src/App.js

When I want to manage the pagination and then pass the data to the table it breaks and show more rows than expected.
It used to work on version 3.0.17

Steps to reproduce the behavior:

1. Go to pagination component
2. Change to page 2 and then go back to first one
3. See error

Screenshots
screen shot, on render w
5 rows expected, 5 rows showed

screen shot with the error:
5 rows expected, 8 rows showed

[matthewoestreich]

Thank you for this!

[Domino987]

It looks like you are mixing remote and local data. If you have all the data already, you need to pass it as one array. This allows for correct filtering/sorting/pagination. This mixing dos not work. Please use either local or remote data.

[edgar-arroyo-by]

Hello there @Domino987

I have another case in which this is happening:
https://codesandbox.io/s/changin-array-different-ids-jp9bp?file=/src/App.js

Lets say I have two arrays, and i want to programmatically change the array used as data, if the data inside the arrays do not share the same ids, this duplication error happens again.

import React from "react";
import MaterialTable from "@material-table/core";

function App() {
  const global_cols = [
    { title: "number", field: "number", minWidth: 140, maxWidth: 400 }
  ];

  const global_data1 = [
    {
      number: "1",
      id: 1
    },
    {
      number: "2",
      id: 2
    }
  ];

  const global_data2 = [
    {
      number: "3",
      id: 7
    },
    {
      number: "4",
      id: 2
    }
  ];

  const [pdata, setPData] = React.useState([]);
  const [switcher, setSwitcher] = React.useState(true);

  React.useEffect(() => {
    if (switcher) setPData(global_data1);
    else setPData(global_data2);
  }, [switcher]);

  return (
    <>
      <button onClick={() => setSwitcher(!switcher)}>Cambiar</button>
      <MaterialTable title="Basic" columns={global_cols} data={pdata} />
    </>
  );
}

export default App;

[edgar-arroyo-by]

Lets say I have a promise that I resolve on component mount, lets say I save the resulted-array in the state and I used in the table.
Say I have a button, to recall the promise with different parameters, or to change the called promise.

It is expected that, if the data inside the resulted-array do not share ids (resulted array that i am resolving before injecting that to the table), it will duplicate?

thanks in advance

@Domino987 @oze4 @edgar-pozadas-by

[edgar-arroyo-by]

Does @material-table/core has opinion on the data ids or in how that arrays are generated?

[Domino987]

Hmh, seems weird.
I think this might be caused by prevDataObject[row.tableData && row.tableData.id] and shares data between objects that is not supposed to be shared. I will remove this and only listen to row.id, which should remove the problem.
Can you check once we release a new version?

[edgar-arroyo-by]

yes sir!
thanks a lot!

[matthewoestreich]

@Domino987 I just tested removing that locally, and it does not resolve the error. I imported the CodeSandbox locally to the demos.

From what I have read, it doesn't look like using an index for the key is recommended, and may be why this is happening. For example, if I change the key to the following it works just fine (no error):

key={'row-' + data.tableData.id + '-' + Math.floor(Math.random() * 100)}

Perhaps we should create a new prop on the tableData object, tableData.key or whatever, and actually generate a small unique GUID for each row that we can use internally?

[edgar-arroyo-by]

Just adding an (unwanted) opinion:

Removing prevDataObject[row.tableData && row.tableData.id] does not solve the issue because the id being used is the index
As @oze4 says, the issue is on using the index as key. I believe the best way is to generate that small unique GUID and used internally, so the table has no opinion on how the ids of the data should be.

[matthewoestreich]

All opinions are wanted!

I just pushed a commit which contains the added demo for this, if you would like to test locally @Domino987 I called the demo 'DataSwitcher' or something along those lines.