Open DanHarman opened 7 years ago
Hi,
We do not allow binding data to a specific table row.
Our Grid however has the getCellValue
property for providing custom access to column data for a specific row. So, you can pass an array of row identificators to the Grid's rows
property and define the getCellValue function.
I've created an example where my Grid has 100K rows, and each millisecond a random row changes: https://stackblitz.com/edit/react-75nrwr?file=index.js. Make sure to open the demo result in a separate window (the "Open in New Window" button in the upper-right corner) and read notes inside the example code. The main drawback of the provided solution is that React performs virtual dom comparison for each rendered cell.
Note that this solution is not applicable to a Grid with local data processing as these plugins manipulate provided rows. In such scenarios, you should apply sorting and filtering manually to achieve better rendering performance.
I should also mention that this scenario is not well optimized for the Grid (virtual dom comparison), but it will perform way better than the scenario with an array passed to the Grid's rows
property recreated each time the row changes. So, I will mark this issue as an enhancement and you can track its progress here.
Hi,
Thanks a lot for looking into this, I had experimented with that approach but obviously not optimal as the grid is still having to do a list comparison on the row key list.
The local data processing absence is also a really big issue. Are the LocalFiltering
, LocalGrouping
& LocalSorting
really unable to work with this approach as they all seem to import getCellValue
according to your docs and a quick look at filteredRows in dx-grid-core seems to use it?
Finally, not sure I understand why the create function has to be made each render?
// NOTE: it is required to create function each render
const getCellValue = (row, columnName) => data.byId[row][columnName];
Why can't this just be a class member of the component defined as:
private getCellValue = (row: string, columnName: string) => {
return this.state.data.byId[row][columnName];
}
This should at least save on creating a new closure each render?
The getCellValue function should be recreated every time the component is rendered (or a related data change takes place) because our Grid uses shallow comparison in order to improve the rendering performance. So, moving the function to the class level will lead to skipping changes in the control rendering.
As for local data processing plugins. The main idea behind the approach with a normalized state is to separate row list changes (row adding/deleting/moving) and row data changes (row updating). So, adding these plugins will break this mechanism as the row list and row data will be merged again because, for example, local filtering will calculate new rows based on the row data. So, the Grid table will be fully rendered each time the row data changes.
@kvet I've just discovered that you have a table row template feature. Would it not be possible for this to be a connected react element that subscribes to the store for its particular row as per the architectural pattern from Dan Abramov? I've not tested this yet, but it seems like it should work?
I am afraid that it is not possible to connect row data within the tableRowTemplate template and display connected data in the nested cells. As I said earlier, you can provide the getCellValue
property to connect row data directly to cells.
@kvet how come it is not possible? Even if the getCellValue doesn't use anything connected onto the row, as long as the template itself was connected it would cause a row redraw if the data changed for a displayed row?
If there is a way to get this approach working your grid would support very quick updates only invalidating the currently rendered rows (although I can see it would mess up sorting/grouping, but no one should be sorting/grouping by fast ticking cols, so we would need a way to exclude certain columns from sort/grouping).
I had trouble with the solution as written when a row went into editing mode. The getCellValue method is being called with the row data flattened which caused the id to be an array of characters. I was able to work around this by actually making the row data be an array of { id: id, data: data } where data is a structure containing my row data. In this case, I didn't need the map since the row being passed in also contained the row data.
Hello,
We are also currently evaluating your product as a replacement for our custom grid or react-data-grid, and indeed the lack of being able to connect a row to the store is a big drawback from our current store implementation (which is also following Dan Abramov recommendations)
Would you have a way at all to provide this?
Hi, @KJoyner, the data binding method I described earlier is not a complete solution. It may have limitations. As for the whole approach of binding data to a row instead of the Grid itself, let me describe our idea. The grid is not a simple component. It's a tree of components. So, row data can be bound to several sub-components in the Grid (an ordinary table row and an editing table row, for example). It can also be used with local processing plugins that sort, filter, group and paginate rows. So it is required to have row data instead of just row ids. Of course, we can provide an ability to bind row data to a component directly. But in this case, it will not be possible to use local processing plugins as there will be no required row data in the Grid. It will also require wrapping each template that needs row data. It may be not an easy task for users. As an alternative solution, we may provide the getRowData function. It will not allow binding data to a specific component but will help to maintain great performance for frequent live updates. The only thing it will improve is that editing will work correctly. We will appreciate it if you share your thoughts about possible solutions.
@kvet What about allowing to have such component with the price of not using local processing, as a trade off. and the provide the getRowData API as an alternative.
Do you foresee any restricition to use getRowData? In pour current investigation, this it the technical part we need to solve, to be able to provide the binding between the row id and the rowData required by the devExpress grid.
If the API was to exists, we could just delegate the fetch to it without having to hack around it
@kvet Hi, just running through my thinking here to see how it relates to your propoals:
we know for performance reasons it would be nice if we could bind visible rows to the store, so that if there is a redux update, that row will get the updated row data. This would be very performant for ticking prices etc as most of the updates will apply to rows that are not bound so won't cause any overhead beyond updating the redux store.
this approach is not good for filtering/sorting etc as these processes require the full data set. However could we not make it so that the filter/sort do not update unless the user interacts with the filter/sort controls on the screen, or the developer triggers a refresh? This way we can keep use of the integrated features but not have to retrigger updates unless needed?
So we would give the grid the getRowData method, a sorted list of keys, but not have all the problems you outline with having to update all the templates etc. You would simply use getRowData for every key when the filters or sorting is changed, or the developer calls refresh on that plugin.
Hi @DanHarman,
The Grid's integrated filtering and sorting don't process data if filtering or sorting is not set. I mean if filters or sorting is an empty array.
If filtering or sorting is set and data recalculation is skipped, we cannot guarantee that correct rows will be shown. So, I don't understand the idea.
Of course, the capability to bind visible rows to the store is a good solution for the frequent data updates scenario. For that reason, I advised using the getCellValue
method. It works like the getRowData
method.
I've tried the getCellValue
method in a real-world scenario and it seems to work well. I have about 20 visible rows from 1000 and the data refresh does not trigger data recalculation within 980 invisible rows. The other 18-20 rows are not rendered by the React reconciliation. I assume that this is enough for the most cases. As I understand, you want to fully eliminate data recalculation and React reconciliation for invisible rows, do you?
@kvet Hi, the thing with losing filtering and sorting is that often, the rows being sorted or filtered by are not the ones updating frequently. Why can the plugins not just call getCellValue
but allow the developer to trigger when they update?
@DanHarman, the Grid is created as a normal React component. It doesn't have methods. So, we will not provide such a feature in the future. I'm afraid that the only way to optimize your scenario is to filter, sort and paginate data outside of the Grid. You will receive full control on updating visible rows and filtering/sorting/paging data process.
@kvet For this scenario surely you could put a method on the grid that could be accessed via ref?
Or perhaps provide reference implementations for the other features and how to integrate them. As a customer it feels like a lot of work to implement those features from scratch.
Hi its cool and solve my problem of performance. but i cant use CustomTreeData on getChildRows const getChildRows = (row, rootRows) => { }; row is always null
can you build some basic example how create Tree Data using getCellValue thank you
i build this sample code can you say if its best performance case sample
how i implement sort and filter ? when i use getCellValue?
Hi,
Do you have a recommended approach for frequent live updates to a large number of rows? e.g. live stock price updates for several thousand rows?
The recommended approach from Dan Abramov is to make the store fully normalised e.g.:
i.e. separating out the list of item from the items. themselves The principal being that if you are binding a grid then the grid component would be
connect
ed toallImages
and then each visible row component would beconnect
ed to the store directly so they can do a cheap reference comparison of their row data to detect changes to their particular entity viamapStateToProps
. This means the grid only needs to consider rows that have changed rather than all rows.e.g.: (and excuse the data model change, I'm pasting from differing examples)
This is more fully explained here https://github.com/reactjs/redux/issues/1751 and a fuller example can be found here: https://github.com/mweststrate/redux-todomvc. These are where I lifted the example code from.
Anyway in the context of your ReactGrid, I don't really see how to use this approach since there doesn't appear to be a row component to connect, so I would have to modify the rows property every time. This would seem to imply you need to do a list comparison on every update which is going to be expensive.
Thoughts and recommendations much appreciated. At the very least, is there a way to efficiently communicate a row update to the grid?