[DataGridPro][Exploration] Lazy load tree data

[MBilalShafi]

Initial exploration

TODOs:

• Extend fakeServer to support lazy-load row children
• Extend fakeServer to support filtering for lazy-load row children
• Extend fakeServer to support sorting for lazy-load row children
• Add server-side filtering support to the demo
• Add server-side sorting support to the demo
• Rebase master
• Introduce dataSource
• Optimize the rowTreeCreation
• Add support for defaultExpansionRowDepth

https://deploy-preview-9150--material-ui-x.netlify.app/x/react-data-grid/tree-data/#children-lazy-loading

[mui-bot]

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-9150--material-ui-x.netlify.app/

Updated pages

• docs/data/data-grid/tree-data/tree-data.md

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	705.6	1,090.7	798.9	890.22	135.865
Sort 100k rows	ms	617.5	1,092.9	1,092.9	913.82	186.782
Select 100k rows	ms	200.8	291.7	258.7	252.92	34.471
Deselect 100k rows	ms	149	279.7	187.2	198.7	47.658

Generated by 🚫 dangerJS against 2928687

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[cherniavskii]

Good initial work! 👍
Adding a few comments to continue the discussion around the server-side data fetching topic

[cherniavskii]

The next step for this demo is to support server-side sorting and filtering.
When using server-side data fetching, the sorting and filtering should be automatically in “server” mode, and the models should be passed to the function that fetches the rows.

[MBilalShafi]

the sorting and filtering should be automatically in “server” mode

In start I wanted to keep the API consistent by having the users pass filterMode="server" and sortMode="server" explicitly for filtering and sorting to work server-side. Or we may have to apply the same to the single-level lazy loading.

But it makes sense to do so too as there's not much value in client-side filtering/sorting with server-side data.

[cherniavskii]

When using client-side tree data, we use getTreeDataPath to tell the grid how to create the tree structure.

When fetching rows on the server, I think we don’t need getTreeDataPath anymore.
The grid fetches the top-level rows and creates the tree structure based on the returned rows. From isServerSideRow we know which rows have children, and once the parent row is expanded and the children rows are fetched - we know where to put them without getTreeDataPath. Does this make sense?

[MBilalShafi]

I had an impression the user can provide rows in the beginning in these possible ways:

1. No rows at all, in this case, the grid does a call on the mount to onFetchRowChildren (or whatever the fetcher function is) with path: [] as a parameter to fetch root level rows.
2. Top-level rows only
3. Some initial rows, not necessarily on the top level

So it doesn't necessarily have to be top-level, also, the server should have stored the path in the database to be able to reflect the requests containing request payload similar to:

{
  path: ['level 1', 'level 2', ...],
  ...otherProperties
}

So it should be possible for the BE servers to return the path with each row whose key the user can define using getTreeDataPath. Do you see another benefit of handling this internally other than the user not having to provide an extra prop?

[cherniavskii]

Options (2) and (3) combine both client-side and server-side data fetching, right? I don't think we should mix these fetching strategies. If you have some data cached on the client already, you should be able to skip the server request in onFetchRowChildren and return the cached data immediately.

Do you see another benefit of handling this internally other than the user not having to provide an extra prop?

My main point was avoiding client-side row grouping in the server-side mode because the tree structure is already known.

[cherniavskii]

Rename isServerSideRow to hasChildren? To be more explicit about the purpose of this function

[romgrk]

I like having "server side" in the name of that prop though, makes it easy to understand its purpose.

[cherniavskii]

Maybe we can encapsulate the data source in a single interface instead? Like unstable_dataSource, that will implement an interface like this or similar:

interface DataSource {
  /**
   * This effectively replaces `getTreeDataPath`.
   * Consider this structure:
   * - (1) Sarah // groupKey 'Sarah'
   *   - (2) Thomas // groupKey 'Thomas'
   * When (2) is expanded, the `getRows` function will be called with group keys ['Sarah', 'Thomas'].
  */
  getGroupKey(row): string;
  hasChildren(row): boolean;
  getRows(params: {
    sortModel: GridSortModel;
    filterMode: GridFilterModel;
    groupKeys: string[]; // array of keys returned by `getGroupKey` of all the parent rows
    // In the future, we'll also pass pagination and lazy loading info here
  })
}

<DataGrid
  treeData
  unstable_dataSource={dataSource} // This automatically means server-side row fetching, filtering, sorting, etc.
/>

What do you think?

[MBilalShafi]

I was thinking about something similar but embedding hasChildren and getGroupKey inside the dataSource seems an interesting idea. Thanks for the useful feedback!

[cherniavskii]

embedding hasChildren and getGroupKey inside the dataSource seems an interesting idea.

I'm not 100% about this, but so far I couldn't find a reason to keep them in the root props - therefore I moved them to the data source where they seem to belong. This might change though 🙂

[cherniavskii]

From the DX perspective, would it be easier to return a Promise instead of callbacks? This won't work when emitting fetchRowChildren event, but I think we can remove this event and keep the function signature only

[MBilalShafi]

What about the errors, if we return a Promise, how can we allow to handle the errors in userland?

[cherniavskii]

You can still use try/catch:

async function onFetchRowChildren({ row }: GridFetchRowChildrenParams) {
  try {
    const childRows = await fetchRows(row);
    return childRows;
  } catch (error) {
    // handle error
    throw error;
  }
}

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[MBilalShafi]

Followed up by #12317