Skip to content
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

[DataGridPro] Server-side tree data support #12317

Open
wants to merge 67 commits into
base: master
Choose a base branch
from

Conversation

MBilalShafi
Copy link
Member

@MBilalShafi MBilalShafi commented Mar 4, 2024

Part of #8179
Resolves #3377

Previews:
Server side tree data with server-side indexed pagination, sorting, filtering, and caching: https://deploy-preview-12317--material-ui-x.netlify.app/x/react-data-grid/server-side-data/tree-data/
Plain data server-side pagination, filtering, and sorting: https://deploy-preview-12317--material-ui-x.netlify.app/x/react-data-grid/server-side-data/#usage

Action items:

  • A basic data source implementation via a dedicated hook
  • New strategy processor for server-side tree data
  • Allow use of the cache with the data source
  • Make pagination.rowCount a controlled state (extracted to [DataGrid] Make rowCount part of the state #12381)
  • Support x-data-generator to provide server-side filtered and sorted data (mock an actual data source) => (useDemoDataSource)
  • Support server-side sorting, filtering, and (index-based) pagination.
  • Make rows an optional prop (extracted to [DataGrid] Make rows an optional prop #12478)
  • Extract cache into separate plugin + support imperative operations on it
  • Support an internal cache
  • Support unknown and estimated rowCount (extracted to [DataGrid] Support advanced server-side pagination use cases #12474)
  • Support defaultGroupingExpansionDepth and isGroupExpandedByDefault
  • Demos
    • Error handling (plain data)
    • Error handling (tree data)
    • Custom cache on plain data
    • Demos with swr and react-query
  • Discoverability: Make the demos closer to the actual code to make it easy to copy-paste
  • Perf: Optimize the rowTreeCreation and other processes of rows pre-processing
  • Revisit naming conventions (e.g. DataSource vs DataManager vs DataProvider vs ?)
  • Make loading a controlled prop ?
  • Test coverage

Follow-up Items (for subsequent PRs):

@MBilalShafi MBilalShafi added component: data grid This is the name of the generic UI component, not the React module! feature: Server integration Better integration with backends MUI X Pro labels Mar 4, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 10, 2024

This comment has been minimized.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 10, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 12, 2024

This comment has been minimized.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 22, 2024

This comment has been minimized.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 29, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 5, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 16, 2024

This comment was marked as outdated.

Comment on lines -640 to +660
logger.debug(`Updating all rows, new length ${props.rows.length}`);
logger.debug(`Updating all rows, new length ${props.rows?.length}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidenote but these logger calls have the disadvantage that we need to create a new string in memory every time it runs, even if it doesn't log anything. If it's in a hot path, it could affect performance. A better option would be printf style calls that don't allocate new strings in the arguments, e.g. printf('new length: %i', props.rows.length).

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 3, 2024
packages/x-data-grid-pro/src/models/dataGridProProps.ts Outdated Show resolved Hide resolved
import { GridGetRowsParams, GridGetRowsResponse, GridDataSourceCache } from '../../../models';
import { GridDataSourceCacheApi } from './interfaces';

class SimpleServerSideCache {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this simple cache is enough for demo purposes, we should provide a more advanced cache implementation required for real-world use cases – at least with configurable cache time.
What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make the default cache as simple as possible, but I agree ttl could be added, as it's one of the most basic features of a cache. Added!
Let me know if it feels better now.

Copy link

github-actions bot commented Jun 5, 2024

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Server integration Better integration with backends plan: Pro Impact at least one Pro user PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Implement server-side data source in tree data
6 participants