gregnb / mui-datatables

Datatables for React using Material-UI
MIT License
2.71k stars 934 forks source link

Reactive data causing table state to be lost #729

Closed kpervin closed 4 years ago

kpervin commented 5 years ago

Currently using Meteor to generate reactive data, and if I change any table settings (filter, sort, etc), they are lost if any of the data is updated. Previously, I thought it was because there were functions in options and columns that were updating the props, but it seems even the data prop, if updated, causes the state to be lost (for me). A sample of my code:

import { LabClinics } from "/imports/api/labclinics/labclinics";
import ReactSelect from "/imports/ui/Components/ReactSelect.js";
import {
    Button,
    Grid,
    IconButton,
    Paper,
    TextField,
    Toolbar,
    withStyles,
} from "@material-ui/core";
import CheckIcon from "@material-ui/icons/Check";
import { Meteor } from "meteor/meteor";
import { withTracker } from "meteor/react-meteor-data";
import MUIDataTable from "mui-datatables";
import React from "react";
import { toast } from "react-toastify";

class UsersComponent extends React.PureComponent {
    constructor(props) {
        super(props);
        this.state = {
            data: [],
            users: null,
            options: this.returnOptions(),
            columns: this.returnColumns(),
            clinic: null,
        };
        this.onSubmit = this.onSubmit.bind(this);
    }

    componentDidUpdate(prevProps, prevState, snapshot) {
        if (this.props.isReady && this.props !== prevProps) {
            this.createRows();
            this.createClinicSelectOptions();
        }
    }
    onChange(e) {
        console.log(e.target.value);
        this.setState({ [e.target.name]: e.target.value });
    }

    createClinicSelectOptions = () => {
        const clinicOptions = this.props.clinics.map((clinic) => ({
            label: clinic.AurusUser,
            value: clinic,
        }));
        this.setState({ clinicOptions });
    };

    onChangeClinic = (value) => {
        this.setState({ clinic: value });
    };

    returnOptions = () => ({
        onRowsSelect: (currentRowsSelected, allRowsSelected) => {
            if (currentRowsSelected.length <= 0) {
                this.setState({ clinic: null, users: null });
            } else {
                const users = [];
                currentRowsSelected.forEach((row) => {
                    users.push(this.state.data[row.dataIndex][0]);
                });
                this.setState({ users });
            }
        },
        customToolbarSelect: (selectedRows, displayData, setSelectedRows) => {
            return (
                <Toolbar
                    style={{ flexGrow: 0.1 }}
                    className={"toolbar-spacing"}
                >
                    {/*<Typography>Reassign User to Clinic:</Typography>*/}
                    {this.state.clinic && (
                        <React.Fragment>
                            <IconButton onClick={this.onSubmit}>
                                <CheckIcon fontSize={"small"} />
                            </IconButton>
                        </React.Fragment>
                    )}
                    <ReactSelect
                        options={this.state.clinicOptions}
                        placeholder={"Reassign User to Clinic"}
                        // label={"Reassign User to Clinic"}
                        helperText={"Reassign User to Clinic"}
                        onChange={this.onChangeClinic}
                    />
                </Toolbar>
            );
        },
        elevation: 1,
    });

    createRows = () => {
        const data = this.props.users.map((user) => [
            user,
            user.profile.username,
            user.profile.clinicId,
        ]);
        this.setState({ data });
    };

    returnColumns = () => [
        {
            name: "User",
            options: {
                display: "excluded",
            },
        },
        {
            name: "Username",
        },
        {
            name: "Clinic ID",
        },
    ];

    render() {
        const { classes } = this.props;
        const { data, options, columns } = this.state;
        return (            
            <MUIDataTable
                title={"Users"}
                data={data}
                options={options}
                columns={columns}
            />            
        );
    }
}

const styles = (theme) => ({
    iconButton: {},
    iconContainer: {
        marginRight: "24px",
    },
    inverseIcon: {
        transform: "rotate(90deg)",
    },
    textField: {
        marginLeft: theme.spacing(1),
        marginRight: theme.spacing(1),
    },
    paper: {
        ...theme.mixins.gutters(),
        paddingTop: theme.spacing(2),
        paddingBottom: theme.spacing(2),
    },
});

export default withTracker((props) => {
    const handles = [
        Meteor.subscribe("labClinics"),
        Meteor.subscribe("userList"),
    ];
    const isReady = handles.every((e) => e.ready());
    let clinics, users;
    if (isReady) {
        clinics = LabClinics.find().fetch();
        users = Meteor.users.find().fetch();
        debugger;
    }
    return {
        isReady,
        clinics,
        users,
    };
})(withStyles(styles)(UsersComponent));

Expected Behavior

Table state should be retained (filters, search text, etc) should any prop update on the MUIDataTable.

Current Behavior

All state resets if any changes in props are made (denoted by "propsChange" in onTableChange).

Steps to Reproduce (for bugs)

  1. Created new Meteor project and populated with test data.
  2. Fed data into datatable.
  3. De-selected a column from "View Columns".
  4. Opened database editor and changed a value in the database to see if reflected reactively in table.
  5. Witnessed the de-selected column then become visible again on data change.

Your Environment

Tech Version
Material-UI 4.1.3
MUI-datatables 2.5.1 (but tested it on 2.4.0, 2.3.0, and 2.2.0
React 16.8.6
browser Chrome Version 75.0.3770.100 (Official Build) (64-bit)
OS MacOS 10.14.5
gabrielliwerant commented 5 years ago

As this is a react library, use with other frameworks like Meteor is out of scope for bug fixes. Can you generate an example failure case without using Meteor? If so, I will look into it.

buddies2705 commented 5 years ago

I can validate this Bug, A propsUpdate event is triggering and resetting table state.

miladehghani commented 5 years ago

I can validate this Bug too, propsUpdate event reset table states.

kpervin commented 5 years ago

@gabrielliwerant sent me this email not long ago when I asked him about the issue:

Ok, so there's a lot going on with this issue behind the scenes. I have a description with more information here if you're interested: /issues/679. The short story is react can't both manage internal derived state and changes passed in via props. It's a one or the other type of situation, so there are a number of bugs due to users wanting one or the other, and so it gets switched back and forth. Recently, I made a switch (in 2.5.1) for the table to forget internal state when it comes to column data. This is the direction the library will be moving in general because it affords the greatest amount of customization possible. If you switch back to 2.5.0, it may temporarily offer you relief from the issue. Another thing you can do is to manage your own state. So whatever state you are losing when props update, make sure to track on your end, and pass those back into the table whenever you make another change that would result in the table forgetting them. Certainly more work than before, but should be possible to fix on your end. Or stick with 2.5.0 as long as you can. Hope that helps. Best, Gabriel

@gabrielliwerant would it be ok if we can get a feature on the table that allows us to flag if internal state should be purged on props update? Or conversely a flag that indicates to retain internal state?

Fabbok1x commented 5 years ago

@gabrielliwerant : Since there is tableState, is there a way to pass in tableState aswell via options for example? As this doesn't seem documented.

gabrielliwerant commented 5 years ago

@Fabbok1x Most of the table state can be passed in. You can find out what is available in the options in the documentation. Is there something specific you are looking to pass in but can't find an option for?

mdobroggg commented 5 years ago

Seeing this as well. Seems to happen when I enter search text. The text immediately gets deleted.

Screen Shot 2019-08-01 at 5 02 59 PM

Passing in my own state (searchText) works here, but it causes the textbox to render text only after you stop typing. Rolling back to 2.5.0 didn't change anything.

gabrielliwerant commented 5 years ago

Hello @mdobroggg. In order to debug this, I need to see a minimal example of your code. There are too many possibilities to determine what is causing your issue from the screenshot provided, and even whether or not it is caused by the same thing as anyone else here.

In fact, this issue was opened due to something specific occurring with meteor. I'm going to close this issue as it doesn't appear relevant to the project (meteor is not supported/guaranteed to work). If you can reproduce a minimal example without external libraries, please open a new issue with the code sample (ideally in codesandbox, so it is already up and running).

Fabbok1x commented 5 years ago

It'd be great if there was a way to serialize the whole state and pass it as modified object. A example on saving the whole state and passing it would be greatly appreciated.

MasterKale commented 5 years ago

@gabrielliwerant I've managed to reproduce this same issue in a clean Codesandbox environment. You can see the bug in action here:

https://codesandbox.io/embed/reverent-bush-4zv5b

Try to click the columns to change sort order, or change the column to sort by, and you'll see that neither works.

When you attempt to sort, you'll see two onTableChange events occur:

Screen Shot 2019-08-27 at 10 51 08 AM
  1. The first onTableChange action (sort)
  2. An API request is triggered via getContinents()
  3. The second onTableChange action (propsUpdate)

The getContinents() method called in the first onTableChange makes a GraphQL request, which updates the data value created via the useLazyQuery() hook. When this data updates (as in when new data is fetched), a useEffect() gets triggered that formats the new items in data so that they're ready to be displayed in the table. This updates the component's stateful rows variable via setRows(), which causes the second onTableChange to trigger with an action of propsUpdate.

I'm all ears on ways I might re-architect things to work with the current version of MUI-DataTables...

gabrielliwerant commented 5 years ago

Thanks @MasterKale, I'll look into this again when I get a chance.

ronaiza-cardoso commented 5 years ago

Hi, I'm having the same issue, when I make an api when 'changeRowsPerPage': and 'changePage:', propsUpdate is triggered and the value is inside the table is cleaned:

image

MasterKale commented 5 years ago

@ronaiza-cardoso Unfortunately, for now, you have to go all-in if you decide that you want to use server-side pagination/sorting/filtering. I managed to make the switch, though, so here's the gist of how you can handle things until v3 gets released:

NOTE: for a bit of context I'm working exclusively in Hooks so some of my explanation might not work if you're working with Class Components

  1. Set up an onTableChange option when you set up your <MUIDataTable /> that triggers logic to store the table's state in your component's setState()/this.state/etc...:
<MUIDataTable
  columns={columns}
  data={rows}
  options={{
    onTableChange: (action: string, newTableState: MUIDataTableState) => {
      switch (action) {
        case 'changeRowsPerPage':
        case 'changePage':
        case 'sort':
        case 'filterChange':
        case 'resetFilters':
          updateTableState(newTableState);
          break;
        case 'propsUpdate':
        case 'columnViewChange':
        default:
      }
    },
  }}
/>
  1. Set up an updateTableState() method that'll persist a few values from table state:
const {
  // For current column sort
  activeColumn,
  announceText,
  filterList,
  rowsPerPage,
  page,
} = newMUIState;

let order = 'desc';
if (announceText) {
  // MUIDataTable stores sort direction in a string: `Table now sorted by time : descending`
  const direction = announceText.split(' : ')[1];
  order = direction === 'ascending' ? 'asc' : 'desc';
}

setTableState({
  activeColumn,
  order,
  filterList,
  rowsPerPage,
  page,
});
  1. Re-render your columns, paying attention to how sortDirection and filterList get set based on the updated tableState:
const columns = useMemo(() => {
  const {
    activeColumn,
    filterList,
    order,
  } = tableState;
  const [
    filterCats,
  ] = filterList;

  return [
    {
      name: 'cats',
      options: {
        sortDirection: activeColumn === 'cats' ? order : undefined,
        filterList: filterCats,
      },
    },
  ];
}, [tableState]);
  1. Make your API request whenever tableState variable gets updated (by the setTableState() above):
useEffect(async () => {
  const {
    activeColumn,
    order,
    filterList,
    rowsPerPage,
    page,
  } = tableState;

  // Make your API request
  const resp = await axios.get(...);

  // Format the returned data into a data structure the data table will understand
  const newRows = resp.data.map(() => {...});

  // Persist the new rows in state
  setRows(newRows);
}, [tableState]);

When your rows state, which you've set to the data table's data prop, updates, this'll trigger a propsUpdate event. But, since the switch() statement in onTableChange does nothing for that particular case, your table will function as expected.

It's a lot of boilerplate but that's just the reality of server-side rendering. Give it a try!

mdobroggg commented 5 years ago

@MasterKale I think your answer gets some of the basic functionality like pagination working, but I'm still seeing issues with filtering, sorting, and view columns not maintaining state between updates.

I've found any re-render will disrupt the state. This includes updating a state that the table doesn't use or calling this.forceUpdate(). This can be reproduced by modifying the table state in someway such as changing the columns to view and then calling this.forceUpdate() in any listener such as onColumnSortChange. You will see the table's shown columns reset, the sort arrow will never appear and a propsUpdate action will fire despite none of the props to the data table actually changing.

Happy to help however I can with getting this resolved.

gabrielliwerant commented 5 years ago

Hello @mdobroggg. I actually don't think this is resolvable without an entirely new API, which is planned, but is not short term. The fundamental problem is that the table doesn't know your intent when it re-renders. It doesn't know if you meant to initialize based on what you passed in via props, or with its own state. In most cases, it is reverting to passed in props because this offers the most opportunities for devs to manage their own table state. Essentially, this is the direction the table is going to go in, otherwise the ability of developers to customize the experience will be much more limited.

So, if you want to keep your state between re-renders, you have to manage that state yourself and pass it in via props. You should be able to do this with just about everything, including sortDirection for the sort arrow icon and filterList for active filters, both in column options.

MasterKale commented 5 years ago

@mdobroggg I apologize for the confusion! It turns out I missed a step that involves re-building the columns prop to take into account the tableState's filterList, activeColumn, and order to properly persist changes made by clicking to sort and filter by a given column.

Take another look at the new Step 3 I just added - I think it'll be the missing piece of the puzzle!

mdobroggg commented 5 years ago

@MasterKale Thanks a lot! That did end up working with a few tweaks and I had to do the sorting and filtering manually. Here is my code in case someone else needs a quick sort / filter.

NOTE: not using hooks so this will differ from masterKale's samples

sortAndFilterData = (currentData, tableState) => {
    if (!tableState) {
      this.setState({
        data: currentData,
        displayData: currentData,
      });
      return;
    }
    // sort
    const {
      columns, activeColumn, order, filterList,
    } = tableState;
    const data = [...currentData];
    if (activeColumn !== null) {
      const activeColName = columns[activeColumn].name;
      if (order === 'desc') {
        data.sort((a, b) => {
          if (a[activeColName] < b[activeColName]) {
            return -1;
          }
          if (a[activeColName] > b[activeColName]) {
            return 1;
          }
          return 0;
        });
      } else {
        data.sort((a, b) => {
          if (a[activeColName] > b[activeColName]) {
            return -1;
          }
          if (a[activeColName] < b[activeColName]) {
            return 1;
          }
          return 0;
        });
      }
    }

    // filter
    let filterData = data.filter((d) => {
      for (let i = 0; i < filterList.length; i += 1) {
        const filters = filterList[i];
        const colName = columns[i].name;
        for (let j = 0; j < filters.length; j += 1) {
          const filter = filters[j];
          if (d[colName] === filter) {
            return true;
          }
        }
      }
      return false;
    });

    if (filterData.length === 0) {
      filterData = data;
    }

    this.setState({
      data,
      displayData: filterData,
    });
  }
KrzysztofMadejski commented 5 years ago

Recently, I made a switch (in 2.5.1) for the table to forget internal state when it comes to column data. This is the direction the library will be moving in general because it affords the greatest amount of customization possible.

Sounds like a breaking change. Shouldn't version be bumped to 3.x?

gabrielliwerant commented 5 years ago

@KrzysztofMadejski It isn't quite that simple. The table is currently handling mutually exclusive use cases, so a bug in one thing creates a feature in another, and fixing one can break the other. But if one bug is considered legitimate, then it seems reasonable to me to "fix it" in a non-major version, even if that breaks something for someone else that was relying on the bad behavior. This can always happen, even in patched versions, but I think it's ok to make the distinction between unintended behavior and intended behavior as far as releases go. Any changes I've been making like this were specifically to fix already broken behavior in the areas deemed within scope and intention. Known breakages or changes in API are deprecated first and will not be switched over until a major release.

KrzysztofMadejski commented 5 years ago

That makes sense, there was no promise it will work this way. I used the workaround MasterKale suggested. I keep fingers crossed for the next big release. Cheers!

gabrielliwerant commented 5 years ago

That being said, I will make exceptions if a "fix" seems to break too many things...

huaraco commented 4 years ago

also it happened for the pagination. when i update the data or fetch to update the state, it will back to the first page.

patorjk commented 4 years ago

State in table is persistent as of version 2.15.0.