gregnb / mui-datatables

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

customBodyRender bug #915

Closed PasVV closed 5 years ago

PasVV commented 5 years ago

Hello!

When i trying to provide customBodyRender function in column options its broke filter with that column.

Errors heading looks like:

  1. "Failed prop type: Invalid prop primary supplied to ForwardRef(ListItemText), expected a ReactNode."
  2. "Failed prop type: Invalid prop children supplied to ForwardRef(Typography), expected a ReactNode."
  3. "Invariant Violation: Objects are not valid as a React child (found: object with keys {id, name}). If you meant to render a collection of children, use an array instead."

I trying to use this customBodyRender function:

(field: { id: number; name: string }) => {
  return (
    <span style={{ color: STATUS_MAP[field.name] || 'inherit' }}>
      {field.name}
    </span>
  );
};

When i trying to return only field.name string, without <span></span> wrapper all works fine.

Also, your example works.

The Column:

{
  name: 'status',
  label: 'Status',
  options: {
    customBodyRender: renderStatusLabel,
  }
},

A row data:

status: { id: 1, name: 'Ready' }

What i missed?

Tech Version
Material-UI 4.3.3
MUI-datatables 2.1.8
React 16.8.6
browser any
gabrielliwerant commented 5 years ago

Hello @Lilliance. You list your version of this library here as 2.1.8, is that correct? Can you confirm that the issue still exists in the latest version, which is 2.10.3?

PasVV commented 5 years ago

Thank you for advice, @gabrielliwerant. I updated library to 2.10.3, but i have same issue. :( I suppose, it can be place for my inattentiveness, maybe i missed something obvious.

gabrielliwerant commented 5 years ago

It looks like be using incorrect arguments in the function. Check out the component example in the library. The arguments are value,, tableMeta, updateValue. value is a string, and it is the value intended for the cells in the column you are customizing.

PasVV commented 5 years ago

When i don't use any args, i have same issue. You can reproduce that, if you pass this render fn:

{
    name: 'status',
    label: 'Status',
    options: {
      customBodyRender: () => <span>hello</span>,
    }
  },

And this is cell data:

{
  status: {id: 1, name: 'arrived'}
}
gabrielliwerant commented 5 years ago

I'll need to see an example of your problem with all the (minimal) code necessary to create the issue. Can you create a codesandbox (https://codesandbox.io) and share it here?

PasVV commented 5 years ago

Sure, you can check my example I noticed one more thing: if you set filterType to dropdown - problem solves, but in multiselect or checkbox cases still exists.

gabrielliwerant commented 5 years ago

Thank you! I can reproduce the problem now. Will take a look.

gabrielliwerant commented 5 years ago

Ok, I've got a better handle on this now. I'm downgrading the issue to an "enhancement".

Fundamentally, the table isn't set up to use objects the way you are attempting, so the issue is deeper than filters or customBodyRender. Columns have to be set up to work with single values like strings or numbers. If your data has other objects in it, then you'll need to tell the column which data you want to use in the name field (e.g. name: "field.id", also see the data-as-objects example).

So the problem with the table is that it's allowing the object to propagate through the code until it blows up. I'm working on a PR that I'll mention here when pushed, which will throw an error earlier in the execution of the code so that you and others can see it and deal with it before it becomes some generic react error that's difficult to trace.

PasVV commented 5 years ago

@gabrielliwerant, thank you for explanation, i appreciate your help! I thought about that, but in some cases if i use cell data as object all work fine. But now i see it was just non-documented luck :), ty.

gabrielliwerant commented 5 years ago

@Lilliance That's correct, it looks like you got lucky! I think you would have run into problems eventually. Not your fault, as there were no error messages to help out here, but hopefully the additions will provide something good enough for now.

gabrielliwerant commented 5 years ago

P. S. I was partially incorrect, it looks like array data is allowed for cells, which will be automatically concatenated together, unless you use customBodyRender, in which case, you can drill into value as an array. However, objects don't work.

PasVV commented 5 years ago

Yes, i got it. Also i found one more bug, if i use object as value - broking sort. You was right.

I converted all object values to string and now everything works fine. :)

I thought work with objects save my time with server-side filtering (because i will send entity ids to server), but... :(

gabrielliwerant commented 5 years ago

If you need serverside filtering, I have a PR up to add support for it that you might want to take a look at https://github.com/gregnb/mui-datatables/pull/913.