Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[docs] Improve the docs of updateRows #5949

Open
2 tasks done
oliviertassinari opened this issue Aug 29, 2022 · 9 comments
Open
2 tasks done

[docs] Improve the docs of updateRows #5949

oliviertassinari opened this issue Aug 29, 2022 · 9 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation plan: Pro Impact at least one Pro user v6.x

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 29, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Screenshot 2022-08-29 at 16 59 24

https://mui.com/x/api/data-grid/grid-api/

Expected behavior 🤔

Screenshot 2022-08-29 at 17 02 01

Steps to reproduce 🕹

No response

Context 🔦

This is documented as Pro only in https://mui.com/x/react-data-grid/row-updates/#the-updaterows-method, and fails when called:

const updateRows = React.useCallback<GridRowApi['updateRows']>(
(updates) => {
if (props.signature === GridSignature.DataGrid && updates.length > 1) {
// TODO: Add test with direct call to `apiRef.current.updateRows` in DataGrid after enabling the `apiRef` on the free plan.
throw new Error(
[
"MUI: You can't update several rows at once in `apiRef.current.updateRows` on the DataGrid.",
'You need to upgrade to DataGridPro or DataGridPremium component to unlock this feature.',
].join('\n'),

Your environment 🌎

v5.16.0

Order ID 💳 (optional)

No response

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation component: data grid This is the name of the generic UI component, not the React module! plan: Pro Impact at least one Pro user labels Aug 29, 2022
@flaviendelangle
Copy link
Member

This is documented as Pro only in https://mui.com/x/react-data-grid/row-updates/#the-updaterows-method, and fails when called:

I fails when called with several rows.
The method is not pro-only but it has a limited behavior on community plan.

That's the same topic again, the apiRef is available on community plan, so technically people can use it. However it's only available on slots so in a real scenario, most of the time the apiRef is pro only.
But that's the case for all api method, updateRows is not more pro-only than other methods like setFilterModel

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 29, 2022

The method is not pro-only but it has a limited behavior on community plan.

@flaviendelangle Ah, I didn't notice that there was a valid use case with the community plan. My bad, thanks.

I'm confused then. What's the different with calling updateRows with 1 row, N times (Community) vs. calling updateRows with N rows, 1 time (Pro only)?

@flaviendelangle
Copy link
Member

The performances of calling it N times would be catastrophic

@oliviertassinari
Copy link
Member Author

@flaviendelangle Ok thanks, I feel that I understand the problem space now.

Screenshot 2022-08-30 at 00 29 02

Screenshot 2022-08-30 at 00 31 11

Both handle 1,000 row updates per second. There is a difference, it's 70% faster, sounds like it's worth having in the Pro plan then.

As a follow-up, I would propose:

  1. Complement the TypeScript fail with a runtime warning:
diff --git a/packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx b/packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx
index 7058730b1..11f7ae3ee 100644
--- a/packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx
+++ b/packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx
@@ -1,6 +1,6 @@
 import * as React from 'react';
 import PropTypes from 'prop-types';
-import { chainPropTypes } from '@mui/utils';
+import { exactProp, chainPropTypes } from '@mui/utils';
 import {
   GridBody,
   GridErrorHandler,
@@ -748,3 +748,8 @@ DataGridRaw.propTypes = {
     PropTypes.object,
   ]),
 } as any;
+
+
+if (process.env.NODE_ENV !== 'production') {
+  DataGridRaw.propTypes = exactProp(DataGridRaw.propTypes as any);
+}

Which with https://codesandbox.io/s/peaceful-shannon-qrsfzv?file=/demo.tsx codesandbox would turn into:

Screenshot 2022-08-30 at 00 15 54

  1. In the documentation, to split this h2 https://mui.com/x/react-data-grid/row-updates/#the-updaterows-method into two different h2. We could have one to cover single-row updates and another one for multiple-row updates.
  2. To consider splitting the updateRows methods in two, one for each plan. Not a strong perspective on this, but if we don't, then to update the API description to mention that multiple rows are Pro plan and above only. It's surprising right now for a community user to see that only one row update at a time is supported:

Screenshot 2022-08-30 at 00 36 31

https://mui.com/x/api/data-grid/grid-api/#properties

  1. To rename https://mui.com/x/react-data-grid/row-updates/#high-frequency to the name of the prop, like "## Throttle rows updates". It wasn't clear at first that this section would help my use case.

@flaviendelangle
Copy link
Member

  1. Sure
  2. This would require creating a single row update scenario with an apiRef access from inside a component slot. Our current strategy was to be discrete about the community apiRef since it's not very consistent. But now that we have a doc page describing its behavior, we could probably start documenting it.
  3. Not sure about that one. setFilterModel / setSortModel are also different between pro and community (you can only set one item with the community plan)
  4. I think the term "high frequency" is interesting because it describes the use case, not the technical implementation. In AG Grid they also have high frequency but instead of throttling the update of the rows, they delay the application of the heavy work (sorting / filtering / grouping / ...). If we want to update the name, I would keep "high frequency" somewhere so that people can end up on the page through the Algolia Search with those keywords.

@oliviertassinari
Copy link
Member Author

  1. Agree. For context, I opened this issue because we were talking about exposing the api to the community in v6. It would require to be exposed for this idea to make sense.
  2. Fair, then maybe only a description update would help.
  3. Interesting, to be clear, what got me confused is that we say "high frequency" but we throttle to 2000ms. I was expecting something that would update at a higher frequency 😁. But OK, maybe 4. Isn't good idea.

@flaviendelangle
Copy link
Member

For 4., I think this is a good example of a feature that people can name very differently depending on what they have in mind.

Like for the Master / Detail panel

@oliviertassinari oliviertassinari changed the title [docs] updateRows should be listed as Pro only in the API page [docs] Improve the docs of updateRows Sep 3, 2022
@morganney
Copy link

morganney commented Oct 27, 2024

Seems the docs for updateRows have not been updated yet, or at least I don't see where it mentions updateRows only accepts more than one row for the pro version.

Another thing I noticed about the docs is there is no summary of which features apiRef supports for the base plan vs pro plan vs premium plans. For instance, where does it state in the docs that subscribeEvent is only available under the pro plan? For instance, this issue: #2904

You don't really find out a feature is unsupported by the base plan until you write some code and execute it without complete documentation...

If subscribeEvent is unsupported by the free plan, why even expose the export of useGridApiEventHandler?

@MBilalShafi
Copy link
Member

@morganney I think the subscribeEvent is supported by the free plan for the events that are emitted by the plugins that fall under the free plan, for example try sorting a column here and checking the browser console: https://stackblitz.com/edit/react-vzj41c?file=Demo.tsx

Do you have a particular case where it doesn't work? If yes, could you share a minimal reproduction of the problem here?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation plan: Pro Impact at least one Pro user v6.x
Projects
None yet
Development

No branches or pull requests

4 participants