-
Notifications
You must be signed in to change notification settings - Fork 934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added customBodyRenderLite / removed object deprecation for column values #1339
Conversation
src/MUIDataTable.js
Outdated
@@ -865,7 +871,9 @@ class MUIDataTable extends React.Component { | |||
let columnValue = row[index]; | |||
let column = columns[index]; | |||
|
|||
if (column.customBodyRender) { | |||
if (column.customBodyRenderLite) { | |||
displayRow.push( column.customBodyRenderLite ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove white space
@@ -134,7 +134,7 @@ function TableBodyCell(props) { | |||
className, | |||
)} | |||
{...otherProps}> | |||
{children} | |||
{typeof children === 'function' ? children(dataIndex, rowIndex) : children} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never knew that children could be a function. Interesting.
I thought it was only a react node(element)
Does children mean columns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a pattern called "Function as Child Component". It's useful in certain situations. Here the children equal either the value that will be displayed in the column, or the customBodyRenderLite function, which will generate the value for the column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@patorjk thanks!
This PR:
customBodyRenderLite
customBodyRenderLite is a way of rendering data in a more performant way. Using it can lead to tables that render quicker. Before going further, here is a brief technical description of how customBodyRender works under the covers:
If the data changes, both of the above happen again. This means if you have a table of 1000 rows and 10 columns (all with customBodyRender), A total of 1000 * 10 * 2 = 20,000 function calls get executed related to rendering.
Unlike customBodyRender, the value returned from customBodyRenderLite is not used for filtering. So it's return value has no affect on filters. Instead, filters use the value in the data array. Additionally, unlike customBodyRender, customBodyRenderLite is only called for cells that are visible on the current page. This means for the same table, if you have 10 rows per page, a total of 10 * 10 = 100 function calls get made. A test showing this dynamic has been added.
Another trade off for customBodyRenderLite is that it doesn't take in
(value, tableMeta, update)
, instead it simply takes in(dataIndex, rowIndex)
. This means you have to lookup a value yourself (though personally I've found having the dataIndex is much more useful).Objects no longer deprecated
Back in release 2.12.2, a deprecation warning was added stating that objects would no longer be accepted as values for data (with the exception of arrays). This occurred because of this issue: #915. Essentially what was happening was the filter dialog would cause the table to blow up when given object data.
Forbidding objects is overkill though, as being able to input them is extremely useful. I've patched the error that occurred for the filter dialog. There are two basic problems that remain: