[DataGrid] Improve the invalid `sortModel` and `filterModel` warnings #3671

flaviendelangle · 2022-01-20T10:50:05Z

Part of #3166

For the filtering, we were not blocking the initialization with several items.
For the sorting, we were not blocking neither the initialization nor the control pattern with several items, nor the apiRef call.

I did a console.warn instead of throwing because I can't figure out how to catch the error in the test.
to.throw and toErrorDev don't work when the error is inside a render, and I did not find any example on the core to help me.

What's next

Improve the proptypes check to check both the control model and the initialState sub-key for filterModel and sortModel

mui-bot · 2022-01-20T10:54:47Z

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	236.7	386.3	310.2	309.2	54.555
Sort 100k rows	ms	458.9	791.9	607.6	623.34	109.587
Select 100k rows	ms	139.6	307.5	152.6	182.16	63.128
Deselect 100k rows	ms	88	200.2	199.1	159.92	44.273

Generated by 🚫 dangerJS against a5b545b

packages/grid/_modules_/grid/utils/warning.ts

m4theushw

If you want to throw an error, you can catch it in the test by wrapping the component in an error boundary. You also need to mock console.error to prevent React from logging it.

The prop-types could still be used but it would be better to migrate them to a different file to able to test them with PropTypes.checkPropTypes. I had to remove PropTypes.checkPropTypes in #2395 (comment).

packages/grid/x-data-grid/src/tests/filtering.DataGrid.test.tsx

github-actions · 2022-01-26T12:51:53Z

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

packages/grid/_modules_/grid/hooks/features/filter/gridFilterUtils.ts

packages/grid/_modules_/grid/hooks/features/filter/useGridFilter.ts

packages/grid/_modules_/grid/hooks/features/sorting/gridSortingUtils.ts

github-actions · 2022-01-27T19:34:58Z

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

github-actions · 2022-02-01T10:29:46Z

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

packages/grid/_modules_/grid/hooks/features/sorting/gridSortingUtils.ts

packages/grid/_modules_/grid/hooks/features/filter/gridFilterUtils.ts

m4theushw · 2022-02-11T14:22:32Z

packages/grid/_modules_/grid/hooks/features/filter/gridFilterUtils.ts

+      filterModelMissingItemIdWarning();
+    }
+
+    if (hasItemWithoutOperator || hasItemsWithoutIds) {


Should we make operatorValue required in v6? If yes, then makes sense to also add a warning for it.

I would say yes, I don't like when we are doing magic like adding a default operator.

@alexfauquette if you have a thought on this

Yes, we could make it required.

If it does not cause a problem, I would be in favor of keeping the default operator, just to save the ass of developers that miss use the grid in production

As I see it, it would be like the ID

The typing says it's required

If none is provided, we fire a warning and provide a default behavior

I added a line in Preparing v6 such that we don't forget it

I added a warning + doc line

@m4theushw I kept this one as console.warn because other "deprecation" are warning not error.
But we could change all of them.

packages/grid/_modules_/grid/hooks/features/filter/gridFilterUtils.ts

packages/grid/x-data-grid/src/tests/sorting.DataGrid.test.tsx

…gUtils.ts Co-authored-by: Matheus Wichman <matheushw@outlook.com>

…tils.ts Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

github-actions · 2022-02-14T09:22:33Z

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

packages/grid/_modules_/grid/hooks/features/filter/gridFilterUtils.ts

[DataGrid] Improve the invalid sortModel and filterModel warnings

543c8bc

flaviendelangle added the component: data grid This is the name of the generic UI component, not the React module! label Jan 20, 2022

flaviendelangle self-assigned this Jan 20, 2022

flaviendelangle marked this pull request as draft January 20, 2022 10:53

flaviendelangle changed the title ~~[DataGrid] Improve the invalid sortModel and filterModel warnings~~ [DataGrid] Improve the invalid sortModel and filterModel warnings Jan 20, 2022

flaviendelangle added 2 commits January 20, 2022 11:57

Work

15b5740

Work

a968078

flaviendelangle mentioned this pull request Jan 20, 2022

[docs] Improve sorting documentation page #3564

Merged

4 tasks

Fix

6dfe952

alexfauquette reviewed Jan 20, 2022

View reviewed changes

packages/grid/_modules_/grid/utils/warning.ts Outdated Show resolved Hide resolved

flaviendelangle added 2 commits January 20, 2022 13:41

Fix

23bb123

Merge branch 'master' into sort-filter-warning

f6cc332

flaviendelangle requested review from alexfauquette and m4theushw January 21, 2022 10:35

flaviendelangle marked this pull request as ready for review January 21, 2022 10:35

m4theushw reviewed Jan 21, 2022

View reviewed changes

packages/grid/x-data-grid/src/tests/filtering.DataGrid.test.tsx Outdated Show resolved Hide resolved

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 26, 2022

Merge

73dc09f

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 26, 2022

alexfauquette reviewed Jan 26, 2022

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 27, 2022

Merge

53b666a

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 28, 2022

Fix

3cc6527

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 1, 2022

Merge

e83d42b

flaviendelangle added 2 commits February 11, 2022 11:43

Work

f6fd020

Work

b3c2b3c

m4theushw reviewed Feb 11, 2022

View reviewed changes

flaviendelangle and others added 5 commits February 14, 2022 08:40

Update packages/grid/_modules_/grid/hooks/features/sorting/gridSortin…

b76c436

…gUtils.ts Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Update packages/grid/_modules_/grid/hooks/features/filter/gridFilterU…

55c58fd

…tils.ts Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Update packages/grid/x-data-grid/src/tests/sorting.DataGrid.test.tsx

57726b8

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Code review: Matheus

e5ba2ec

Worké

b20cfa0

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 14, 2022

Fix

16ab9ab

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 14, 2022

alexfauquette mentioned this pull request Feb 14, 2022

[discussion] Preparing v6 #3287

Closed

41 tasks

flaviendelangle added 4 commits February 14, 2022 16:46

Work

7dbaa63

Merge branch 'master' into sort-filter-warning

ed671d8

Code review: Matheus (prepare for operatorValue requirement)

9307f89

Fix

7ec1516

flaviendelangle force-pushed the sort-filter-warning branch from 67644e0 to 7ec1516 Compare February 15, 2022 13:00

flaviendelangle requested a review from m4theushw February 15, 2022 13:36

flaviendelangle added 3 commits February 15, 2022 14:46

Work

8ab1b3d

Work

3d93892

Fix

92fa1ae

m4theushw approved these changes Feb 15, 2022

View reviewed changes

packages/grid/_modules_/grid/hooks/features/filter/gridFilterUtils.ts Show resolved Hide resolved

flaviendelangle added 2 commits February 15, 2022 17:43

Empty

1bb99f0

Empty

a5b545b

flaviendelangle force-pushed the sort-filter-warning branch from 923f63f to a5b545b Compare February 15, 2022 16:49

flaviendelangle merged commit 73ecfcc into mui:master Feb 15, 2022

flaviendelangle deleted the sort-filter-warning branch February 15, 2022 17:54

joserodolfofreitas mentioned this pull request Jul 26, 2022

[data grid] Refactor GridFilterItem properties #5604

Closed

3 tasks

MBilalShafi mentioned this pull request Nov 24, 2022

[DataGrid] Refactor GridFilterItem props #6985

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Improve the invalid `sortModel` and `filterModel` warnings #3671

[DataGrid] Improve the invalid `sortModel` and `filterModel` warnings #3671

flaviendelangle commented Jan 20, 2022 •

edited

Loading

mui-bot commented Jan 20, 2022 •

edited

Loading

m4theushw left a comment •

edited

Loading

github-actions bot commented Jan 26, 2022

github-actions bot commented Jan 27, 2022

github-actions bot commented Feb 1, 2022

m4theushw Feb 11, 2022

flaviendelangle Feb 11, 2022

alexfauquette Feb 14, 2022

flaviendelangle Feb 14, 2022

alexfauquette Feb 14, 2022

flaviendelangle Feb 15, 2022 •

edited

Loading

github-actions bot commented Feb 14, 2022

[DataGrid] Improve the invalid sortModel and filterModel warnings #3671

[DataGrid] Improve the invalid sortModel and filterModel warnings #3671

Conversation

flaviendelangle commented Jan 20, 2022 • edited Loading

What's next

mui-bot commented Jan 20, 2022 • edited Loading

m4theushw left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 26, 2022

github-actions bot commented Jan 27, 2022

github-actions bot commented Feb 1, 2022

m4theushw Feb 11, 2022

Choose a reason for hiding this comment

flaviendelangle Feb 11, 2022

Choose a reason for hiding this comment

alexfauquette Feb 14, 2022

Choose a reason for hiding this comment

flaviendelangle Feb 14, 2022

Choose a reason for hiding this comment

alexfauquette Feb 14, 2022

Choose a reason for hiding this comment

flaviendelangle Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Feb 14, 2022

[DataGrid] Improve the invalid `sortModel` and `filterModel` warnings #3671

[DataGrid] Improve the invalid `sortModel` and `filterModel` warnings #3671

flaviendelangle commented Jan 20, 2022 •

edited

Loading

mui-bot commented Jan 20, 2022 •

edited

Loading

m4theushw left a comment •

edited

Loading

flaviendelangle Feb 15, 2022 •

edited

Loading