gregnb / mui-datatables

Datatables for React using Material-UI
MIT License
2.7k stars 931 forks source link

Access original row object in customBodyRender, etc #1038

Open RyanEwen opened 4 years ago

RyanEwen commented 4 years ago

I would like to feed row-objects into the table and then access them in customBodyRender and other callbacks.

Use case: I need to refer to data in the original row-object that I do not want to use in any column. For example. <Select /> elements with menu items that are specific to the row being rendered. And the data id to update when that <Select /> is changed.

Example data (just one row):

[
  {
    "key": "credit_processor",
    "description": "Which processor integration to use for this client",
    "value": "none",
    "default": "none",
    "options": [
      "none",
      "cardconnect",
      "worldpay",
      "econduit"
    ]
  },
  ...
]

In other libraries I can easily access the original row data to populate my <Select /> and to tell the server what data key is being updated onChange.

Here, I seem to have to make hidden columns with that information, and then keep track of the column indexes containing that data. The code is a lot harder to read and especially hard to maintain (adjusting indexes if new columns are added or removed).

RyanEwen commented 4 years ago

Here's an example of how I am doing things now given my use case above:

import React from 'react'
import FormControl from '@material-ui/core/FormControl'
import MenuItem from '@material-ui/core/MenuItem'
import Select from '@material-ui/core/Select'
import TextField from '@material-ui/core/TextField'
import { withStyles } from '@material-ui/core/styles'
import MUIDataTable from 'mui-datatables'

const styles = {
    main: {
        flexGrow: 1,
        overflow: 'auto'
    }
}

class SettingsTable extends React.Component {
    handleChange = (key, newValue, oldValue) => {
        this.props.onChange(key, newValue)
    }

    renderValue = (value, meta, update) => {
        if (meta.rowData[0] instanceof Array) {
            return (
                <FormControl>
                    <Select
                        style={{ fontSize: 'inherit' }}
                        value={value}
                        onChange={(event) => this.handleChange(meta.rowData[1], event.target.value, value)}
                    >
                        {meta.rowData[0].map((option) =>
                            <MenuItem key={option} value={option}>{option}</MenuItem>
                        )}
                    </Select>
                </FormControl>
            )
        } else {
            return (
                <FormControl>
                    <TextField
                        InputProps={{ style: { fontSize: 'inherit' } }}
                        value={value}
                        onChange={(event) => this.handleChange(meta.rowData[1], event.target.value, value)}
                    />
                </FormControl>
            )
        }
    }

    options = {
        pagination: false,
        selectableRows: 'none'
    }

    columns = [
        { name: 'options', options: { display: false, viewColumns: false, filter: false, sort: false, searchable: false, print: false, download: false } },
        { label: 'Setting', name: 'key', options: { sortDirection: 'asc' } },
        { label: 'Value', name: 'value', options: { customBodyRender: this.renderValue } },
        { label: 'Default', name: 'default' },
        { label: 'Description', name: 'description' }
    ]

    render() {
        let { classes, data } = this.props

        return (
            <main className={classes.main}>
                <MUIDataTable
                    title="System Settings"
                    data={data}
                    columns={this.columns}
                    options={this.options}
                />
            </main>
        )
    }
}

export default withStyles(styles)(SettingsTable)

This is a lot like the workarounds needed with jQuery DataTables before it supported using objects as rows (probably 10 years ago now). Those were dark days :(

gabrielliwerant commented 4 years ago

As discussed here https://github.com/gregnb/mui-datatables/issues/1011, the table has never officially supported passing object data for table cells, and doing so leads to a number of errors if not handled precisely in custom renders. If you're using a custom render, you're stepping into custom territory, and there's not any strong case I can see for accepting objects for this case. As explained in the issue, you can map something like ids back to arbitrary data/objects. They don't need to be passed into the table to get you the information you need.

RyanEwen commented 4 years ago

I'm not sure that you understand. I'm not suggesting that objects be allowed as cell values. My example above does not do that, and there are no warnings generated.

Myself an others would like to have a reference the original row object made available within customBodyRender etc. This reference could be completely ignored by the mui-datatable itself. The only work the table would need to do is to hold the references and pass them to customBodyRender (or just make them available within tableMeta). This is how it works in the other libraries I've used and tested.

gabrielliwerant commented 4 years ago

Hm, you should have access to row data from this function already:

image

Is that not working?

P.S. I apologize if I've misunderstood your issue.

gabrielliwerant commented 4 years ago

It is also might help if I could see your data structure. It doesn't look like you added it to your example above.

gabrielliwerant commented 4 years ago

Oh, this is the row data in the first message. Yes, that's exactly what I'm referring to you not putting into the table directly, instead keeping it separate, and mapping your data back. If you look through the issue I linked here, you will find I modified a dev's codesandbox example to do just that.

RyanEwen commented 4 years ago

The rowData available is your parsed data and not my original data. You do seem to support data in the format I'm sending it in, and you grab the individual strings that are to be shown in the cells by using the name keys I assign, and you have examples of this in the repo. But - there's no reference kept to that original row object, which is what we're after, and what other libraries offer to great benefit.

Is there a reason not to store a reference to the original row object? The way I see it:

gabrielliwerant commented 4 years ago

Yes, there are good reason not to store this data. It's not possible to simply keep the original data untouched, aside from the fact that I don't want arbitrary object passed into the table at all, because otherwise there is no way for me to map the assembled data structure to your original data structure. To do that, I'd have to establish some kind of mapping system and foist it into your data for you (meaning it wouldn't be untouched). But since you already have the data you want, and you know better than the table what shape you want it in, and you know better how you want to map it, this is work that is best left to individual developers. Attempting to map arbitrary object data to the flat structure used by the table is a recipe for bugs and edge cases.

I look at this as a very minor inconvenience in development. Pretty much every app has a specific way it accepts data via its API, so there's often a need to massage your data into a specific shape. And since you can use something like an id to map back to an arbitrary data structure of your own, I don't see any drawbacks with this approach, but lots of drawbacks with allowing and then managing arbitrary object data into the table, which is already causing bugs.

RyanEwen commented 4 years ago

I'm not convinced that you'd need any mapping system or to touch the data. I assume you can add a reference to an existing array item for the row. Or keep a secondary array with the original references that would work with the same index you use with your rowData.

Passing an id into the table is very hacky especially considering the work it takes to hide that id from the user and prevent the table from acting on it via filtering etc. It seems more problematic to me than a reference that can be ignored, plus the workarounds required for that approach end up putting extra work to prevent issues onto the developer's shoulders. Every developer is different, Some are going to do a good job at preventing issues, and others may not. Some might know to turn off filtering on the col, others might not. It will likely be end users who discover these issues.

I almost didn't try this library at all because I didn't see that I could pass in row objects until I looked around a bunch. I wonder how many others just pass on by for that reason, or get as far as I have and then pass :(

RyanEwen commented 4 years ago

Also to touch on the minor inconvenience - the software I work on has many many tables and back in the jQuery days with DataTables.net we had to do what you're suggesting. It may seem minor to you, but it caused many of our developers a lot of grief and there were no shortage of bugs related to having to massage data and map it and reference back to it etc. All of that went away the day we could simply get our original reference in the callbacks. I've been with two companies that lived through that minor inconvenience.. it's not that minor :(

gabrielliwerant commented 4 years ago

Again, I'm not familiar with any API that accepts arbitrary data. The solution I recommend is to map/reduce on your end. The only changes to the table I would consider around this is to provide dataIndex in the rowData so there's a more robust way to refer to the data internally than index which has definite problems as you point out.

RyanEwen commented 4 years ago

If you're not going to change it then I accept that and it's fine. I'll move on to another library.

But when you say you are not familiar with any API that accepts arbitrary data it surprises me as this probably the first library I've used in as long as I can remember that doesn't.

From my experience most libraries prefer that you pass objects as rows (or even require that you use objects). When defining a column they ask you to specify which property name to grab the display value from (or allow you pass a function that returns the display value without the table even knowing which property it came from). They allow you to define custom renderer functions, as mui-datatables does. And some libraries allow you to pass functions that return a different value than the display value to be used during sorting and filtering (in cases where the display value isn't what should be sorted on for some reason). In all of those functions they pass the original row object which includes all the original data passed in regardless of which parts are actually used in the table.

I think a key difference with those same libraries is that they are designed not to touch the passed in row data, while also not copying all of the display values to a new array to use instead of the original data each render. And that is great as that also means they don't duplicate the display values (could be millions of them) and bloat memory. I'm not sure why they would need copy data into another structure anyway, as there are simple strategies to avoid doing that.

For example, I've seen it where the table's internal rows array consists of its own row objects that hold some row state, but most importantly hold a reference to the original row object that was passed in. The row state would be for things like whether the row is shown or selected, etc. The internal rows array, since it's not the literal data array that was passed in to the table, can be sorted however it needs to be and that doesn't touch the original data. The row states can change however they need to be, but the original row data object is left untouched - only being referenced for purposes of getting a display value, or the value to sort or filter on.

// array of row data passed in: 

[
    { id: 1, name: 'Ryan', age: '35', location: { ... }, purchases: [ {...}, {...} ] },
    { id: 2, name: 'Bill', age: '32', location: { ... }, purchases: [ {...}, {...} ] },
    { id: 3, name: 'Randy', age: '51', location: { ... }, purchases: [ {...}, {...} ] }
]

// column definitions:

[
    { heading: 'Name', value: 'name' },
    { heading: 'Age', value: 'age' },
    { heading: 'Location', value: (rowData, rowState) => `${rowData.city}, ${rowData.country}` }
    { heading: 'Purchase Qty', value: (rowData, rowState) => rowData.purchases.length },
    { heading: 'Purchase Subtotal', value: (rowData, rowState) => rowData.purchases.reduce(...) }, // <- some libraries even cache this value in rowState to improve render speed
    { sortable: false, filterable: false, value: (rowData, rowState) => <PurchaseDetailsIcon purchases={rowData.purchases} /> }, 
    { sortable: false, filterable: false, value: (rowData, rowState) => <EditIcon id={rowData.id} /> },
]

// internal table state data: 
{
    sortOrder: [ [0, 'asc'] ],
    searchTerm: 'R',
    columnFilters: [],
    ...
}

// internal row data (sorted by 'name' and searching for the letter 'R' as above):

[
    { 
        data: { id: 2, name: 'Bill', ... }, // <- just a reference
        state: { shown: false, selected: false } 
    }, 
    { 
        data: { id: 3, name: 'Randy', ... }, // <- just a reference
        state: { shown: true, selected: false } 
    }, 
    { 
        data: { id: 1, name: 'Ryan', ... }, // <- just a reference
        state: { shown: true, selected: false } 
    }, 
]
joziahg commented 4 years ago

Why cant we just have a value that acts as an alias for filter, download, print ect. I think by not allowing objects to be passed as data makes this library a lot more cumbersome to work with. Not to mention this feels like it violates DRY principles what with having to make almost 2 identical arrays.

If you are creating a new array for the rows anyways why not just check if the data typeof is an object then grab the alias value from the data instead of the whole data object.

/// rows generation pseudocode

const newRows = passedData.map(data => {
return ( typeof(data) === 'object' ? data.alias : data)
})

Or to make the data more consistent internally, when you create the new array of rows, make all data passed an object putting the data passed as the alias:

// data passed:
const row = [ someString, otherString, { alias: someObjectAlias, extra: someExtraVariable, extra2: someOtherExtraVariable } ]

const newRow = [ { alias: someString }, { alias: otherString }, { alias: someObjectAlias, extra: someExtraVariable, extra2: someOtherExtraVariable } ]

Granted I haven't checked out the source too thoroughly, but is there any reason an approach like this wouldn't work?

spoissant commented 4 years ago

I think the problem is that the concept of the "original row object" simply makes no sense in the context of mui-datatble because the row data it receives is already "digested" (i.e. it has no idea where that data comes from in the first place).

Other libraries (AgReactGrid for example) use a different approach where you provide it the "raw" data and the column definitions contain information on how to retrieve the column data given the raw row data (either in the form of a field name or a callback method).

RyanEwen commented 4 years ago

Yes that's the issue as I see it. Hoping this library can be transformed from former into the latter.

keyvanm commented 4 years ago

A simple use case

Let's say we get an array of user data from the server:

[
   { "avatar": "user1.png", "username": "user1", "name": "John Smith"},
   { "avatar": "user2.png", "username": "user2", "name": "Will Smith"},
]

Now let's say in the table I want to show an avatar of the user next to their username:

const userColumns = (users) => [
  {
    name: 'username',
    options: {
      customBodyRender: (value) => {
        // I have to pass the entire users data, and per each column do  an O(n) operation to find this user
        const user = users.filter( user => user.username === value )[0]; 
        return (
          <span>
            <UserAvatar user={user} />
            <span>{user.username}</span>
          </span>
        );
      }
    },
  }, ...
]

As in the comment in the code, in order to find the avatar src for the user I have to pass in the entire users array and then do a linear search on it to find the user row obj, and grab the avatar src. This operation has an O(n^2) cost to run (O(n) search in each row x O(n) for all rows)

Instead if in customBodyRender we had access to the original row data, I could just grab that and use it, without needing to do a search for it.

What we are suggesting here is adding an extra variable such as originalRowData to the function, and not change anything that is already there. Hopefully this way, people who need this functionality can have access to the original data and nothing else breaks.

Thank you for this amazing library. It really makes handling tables in Material UI much better. I hope you can find the time to take a look at this use case.

gabrielliwerant commented 4 years ago

@keyvanm I don't recommend looking up your values the way that you are doing it. I suggest normalizing your extra data according to id, and using the id as a lookup. The compute time will be constant after that (or nearly so, as I don't think js does hash tables exactly).

So, performance isn't a problem with the do-it-yourself approach. The problem is developer convenience. But I have to prioritize how I spend the limited time I have, and I chose to prioritize issues where the functionality might not be possible at all (or bugs, or security issues, etc.). Since what you and others want here is entirely possible, albeit inconvenient, it's just not going to be a priority for the short to midterm. Perhaps in the future it will be.

doc82 commented 4 years ago

We've noticed some pretty nasty patterns emerging when we want to reference an object or show lots of data in a column (think a list of status-icons)

Would love to see a property with a ref back to the hydrated data model

navinpath commented 4 years ago

I am using MUI Datatables for my react js app but I am not able to get data

acroyear commented 4 years ago

My use case is similar to https://github.com/gregnb/mui-datatables/issues/1038#issuecomment-569106293 - again having to read the contents of 2 columns to decide what to actually show.

https://github.com/gregnb/mui-datatables/issues/1039#issuecomment-674463718

One possibility would be to basically have the model (or rather, the massaged model) itself render the component?

But such a situation (needing to see two or more properties of a row of data to render one column cell) is not at all unusual. Rather, it is a norm in my experience.

Another case is having the type-value-render trio. One column gives the type, another column gives the value, then the renderer looks for the raw value's actual rendered appearance to draw by looking at a key-value mapping (a picklist basically), and it needs the type info to do that look up the right key-value mapping. Again, yes, it could be resolved through massaging the data before going to the table (the "ViewModel" approach of MVVM and the old KnockoutJS library), but in my experience in table libraries for Motif, Java-Swing (and their MVC model), SproutCore, that's not usual. The renderer makes that decision so the model row can be as clean as possible to the original data set.

freewind commented 3 years ago

I was happy when I saw this library until I got stuck in this problem.

juanmartin commented 3 years ago

I just stumbled upon a similar issue in which I needed to pass the "originalRowData" and thought someone might find my solution somewhat useful.

I have my table data stored in context state. const { data, setData } = useContext(dataContext)

In the table options object I have set up a callback click handler: onRowClick: (rowData, rowMeta) => handleClick(rowData, rowMeta)

Then to pass the original labeled row data I use data[rowMeta.dataIndex] inside my handleClick function to push the "original" object from the row the user clicks. This way I do not have to deal with indexes in the state array, instead, I have the object that was used in the table in the first place with key-value pairs.

BTW I'm using history.push('/some/route', data[rowMeta.dataIndex]) to pass data in my route and then useLocation() hook inside the new component.

Hope this makes sense for someone!

ReangeloJ commented 3 years ago

I found a quick workaround. What I have done is, in the columns I added a custom column with the customBodyRender method. Here I pass the tableMeta prop. I then get the current rowIndex and use that index to access the data of that row from the tableData property.

     const columns = [
        {
            name: 'originalData', label: 'OriginalData', options: {
                display: false,
                filter: false,
                sort: false,
                customBodyRender: (value, tableMeta) => {
                    const tableData = tableMeta.tableData;
                    const index = tableMeta.rowIndex;
                    const rowData = tableData[index];
                }
            }
        },
        {
            name: 'name', label: 'Project naam', options: {filter: false, sort: true}
        },
    ]

    const options = {
        renderExpandableRow: (rowData, rowMeta) => {
            console.log(rowData[0]) <-- Has access to the OriginalData property in columns which contains all the original data of that row
        },
    }

I have put my originalData object as first entry in the columns so it's index will always stay 0. Now I can also access this originalData object in my renderExpandableRow method by using the rowData argument and accessing it with index of 0

Hope this helps someone out!!

patorjk commented 3 years ago

For anyone searching who finds this thread, another option is to use the customBodyRenderLite method instead of customBodyRender. That method hands you the dataIndex which you can plug into your data to get the original row data.

porfyriosv commented 3 years ago

Is it okay to use "useContext" inside customBodyRender to get the original data and find the row original data object with index ?

Yashsharma1911 commented 1 year ago

Is this issue fixed now?

BossBele commented 9 months ago

I was happy when I saw this library until I got stuck in this problem.

Me too, it's such an unnecessary problem to have

bloodykheeng commented 4 months ago

@RyanEwen u can access row data directly from the tableMeta argument see how i extracted it and console logged it here down { name: "photo", label: "photo_url", options: { filter: false, sort: false, customBodyRender: (value, tableMeta) => { const tableRowData = tableMeta.rowData; console.log("🚀 ~ ListPage ~ tableRowData:", tableRowData);

      const imageUrl = `${process.env.REACT_APP_API_BASE_URL}${value}`;
      return value ? (
        <Image
          src={imageUrl}
          alt="Item"
          width="100"
          style={{ verticalAlign: "middle" }}
          preview={true}
        />
      ) : (
        <div>No Image</div>
      );
    }
  }
},`

`

RyanEwen commented 4 months ago

@bloodykheeng that property was available back in 2019 when I started this thread, but if you read through it you'll see that it isn't the original row data reference. Not unless there's been a change to the library that hasn't been mentioned in here.