[DataGrid] Fix quick filter input lag #9630

cherniavskii · 2023-07-10T10:09:17Z

I have followed (at least) the PR section of the contributing guide.

Fixes #6783

Before: https://codesandbox.io/s/infallible-villani-gmwpg6
After: https://codesandbox.io/s/long-tree-q7p8c9

mui-bot · 2023-07-10T10:14:26Z

Netlify deploy preview

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

Updated pages

No updates.

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	308.2	415.2	387	363.16	43.973
Sort 100k rows	ms	384	789.3	608.3	604.42	130.642
Select 100k rows	ms	133.4	206.6	165	163.5	25.999
Deselect 100k rows	ms	103.4	246.5	202.6	177.26	58.993

Generated by 🚫 dangerJS against 5c99180

cherniavskii · 2023-07-10T12:38:57Z

packages/grid/x-data-grid/src/components/toolbar/GridToolbarQuickFilter.tsx

@@ -101,7 +101,9 @@ function GridToolbarQuickFilter(props: GridToolbarQuickFilterProps) {

  const updateSearchValue = React.useCallback(
    (newSearchValue: string) => {
-      apiRef.current.setQuickFilterValues(quickFilterParser(newSearchValue));
+      const newQuickFilterValues = quickFilterParser(newSearchValue);
+      setPrevQuickFilterValues(newQuickFilterValues);


The root cause of this issue was here:

mui-x/packages/grid/x-data-grid/src/components/toolbar/GridToolbarQuickFilter.tsx

Lines 88 to 100 in b083f58

React.useEffect(() => {

if (!isDeepEqual(prevQuickFilterValues, quickFilterValues)) {

// The model of quick filter value has been updated

setPrevQuickFilterValues(quickFilterValues);

// Update the input value if needed to match the new model

setSearchValue((prevSearchValue) =>

isDeepEqual(quickFilterParser(prevSearchValue), quickFilterValues)

? prevSearchValue

: quickFilterFormatter(quickFilterValues ?? []),

);

}

}, [prevQuickFilterValues, quickFilterValues, quickFilterFormatter, quickFilterParser]);

Here is what was happening:

type 'a': setSearchValue('a')

debounced setQuickFilterValues(['a']) is called

type 'b': setSearchValue('ab')

useEffect: prevQuickFilterValues /* [] */ !== quickFilterValues /* ['a'] */, reset search value: setSearchValue('a')

type 'c': setSearchValue('ac') // b was lost in the previous step

We should update prevQuickFilterValues as well when updating quickFilterValues internally.

It seems that prevQuickFilterValues should be a ref instead, what do you think? We only use it to ensure that the effect above only runs when quickFilterValues changes.

Good point, it should be a ref indeed!

cherniavskii · 2023-07-10T13:34:36Z

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

+    if (isJSDOM) {
+      this.skip();
+    }
+    // Warning: this test doesn't fail consistently as it is timing-sensitive.


While this test doesn't consistently fail without the fix, I thought it was still better than nothing 🙂
What do you think?

Not ideal but it's fine as it is.

cherniavskii · 2023-07-10T13:35:22Z

packages/grid/x-data-grid/src/components/toolbar/GridToolbarQuickFilter.tsx

@@ -59,7 +59,7 @@ export type GridToolbarQuickFilterProps = TextFieldProps & {
   * @param {any[]} values The new values passed to the quick filter model
   * @returns {string} The string to display in the text field
   */
-  quickFilterFormatter?: (values: GridFilterModel['quickFilterValues']) => string;
+  quickFilterFormatter?: (values: NonNullable<GridFilterModel['quickFilterValues']>) => string;


We never pass undefined as values, so we should exclude it here

github-actions · 2023-07-12T15:48:20Z

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

m4theushw · 2023-07-12T17:26:59Z

packages/grid/x-data-grid/src/components/toolbar/GridToolbarQuickFilter.tsx

@@ -101,7 +101,9 @@ function GridToolbarQuickFilter(props: GridToolbarQuickFilterProps) {

  const updateSearchValue = React.useCallback(
    (newSearchValue: string) => {
-      apiRef.current.setQuickFilterValues(quickFilterParser(newSearchValue));
+      const newQuickFilterValues = quickFilterParser(newSearchValue);
+      setPrevQuickFilterValues(newQuickFilterValues);


It seems that prevQuickFilterValues should be a ref instead, what do you think? We only use it to ensure that the effect above only runs when quickFilterValues changes.

fix quick filter lag

386add2

cherniavskii added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature labels Jul 10, 2023

cherniavskii commented Jul 10, 2023

View reviewed changes

cherniavskii added 2 commits July 10, 2023 15:29

add unit test

0c316e3

fix quickFilterFormatter type signature

df534be

cherniavskii commented Jul 10, 2023

View reviewed changes

cherniavskii requested review from romgrk and m4theushw July 10, 2023 13:35

cherniavskii marked this pull request as ready for review July 10, 2023 13:35

romgrk approved these changes Jul 10, 2023

View reviewed changes

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

m4theushw approved these changes Jul 12, 2023

View reviewed changes

cherniavskii added 3 commits July 12, 2023 21:18

Merge branch 'master' into fix-quick-filter-lag

35943a6

add missing import back

ef6e133

use ref instead of state

5c99180

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

cherniavskii enabled auto-merge (squash) July 12, 2023 19:27

cherniavskii merged commit 4f136ba into mui:master Jul 12, 2023

cherniavskii deleted the fix-quick-filter-lag branch July 14, 2023 11:56

cherniavskii mentioned this pull request Jul 20, 2023

[DataGrid] Lower filter debounce delay #9712

Merged

cherniavskii mentioned this pull request Aug 29, 2023

[data grid] Improve quick filter performance (INP) #9657

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Fix quick filter input lag #9630

[DataGrid] Fix quick filter input lag #9630

cherniavskii commented Jul 10, 2023 •

edited

Loading

mui-bot commented Jul 10, 2023 •

edited

Loading

cherniavskii Jul 10, 2023

m4theushw Jul 12, 2023

cherniavskii Jul 12, 2023

cherniavskii Jul 10, 2023

romgrk Jul 12, 2023

cherniavskii Jul 10, 2023

github-actions bot commented Jul 12, 2023

m4theushw Jul 12, 2023

	React.useEffect(() => {
	if (!isDeepEqual(prevQuickFilterValues, quickFilterValues)) {
	// The model of quick filter value has been updated
	setPrevQuickFilterValues(quickFilterValues);

	// Update the input value if needed to match the new model
	setSearchValue((prevSearchValue) =>
	isDeepEqual(quickFilterParser(prevSearchValue), quickFilterValues)
	? prevSearchValue
	: quickFilterFormatter(quickFilterValues ?? []),
	);
	}
	}, [prevQuickFilterValues, quickFilterValues, quickFilterFormatter, quickFilterParser]);

[DataGrid] Fix quick filter input lag #9630

[DataGrid] Fix quick filter input lag #9630

Conversation

cherniavskii commented Jul 10, 2023 • edited Loading

mui-bot commented Jul 10, 2023 • edited Loading

Netlify deploy preview

Updated pages

cherniavskii Jul 10, 2023

Choose a reason for hiding this comment

m4theushw Jul 12, 2023

Choose a reason for hiding this comment

cherniavskii Jul 12, 2023

Choose a reason for hiding this comment

cherniavskii Jul 10, 2023

Choose a reason for hiding this comment

romgrk Jul 12, 2023

Choose a reason for hiding this comment

cherniavskii Jul 10, 2023

Choose a reason for hiding this comment

github-actions bot commented Jul 12, 2023

m4theushw Jul 12, 2023

Choose a reason for hiding this comment

cherniavskii commented Jul 10, 2023 •

edited

Loading

mui-bot commented Jul 10, 2023 •

edited

Loading