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

[data grid] Lag while typing #5283

Closed
2 tasks done
tonycerdas opened this issue Jun 21, 2022 · 25 comments · Fixed by #5646
Closed
2 tasks done

[data grid] Lag while typing #5283

tonycerdas opened this issue Jun 21, 2022 · 25 comments · Fixed by #5646
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Editing Related to the data grid Editing feature performance

Comments

@tonycerdas
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

when I double-click on a cell that I want to edit when typing the component has a certain delay that erases words and does not allow me to type correctly

Expected behavior 🤔

I wish to write: Hello, how are you?

Datagrid cell: Hllo, ow re yo

Steps to reproduce 🕹

import { Alert, AlertProps, Box, Button, debounce, IconButton, Snackbar, Stack } from '@mui/material';
import { DataGrid, GridCellEditStopParams, GridCellEditStopReasons, GridColumns, GridRowModel, GridSelectionModel, GridToolbar, MuiEvent, useGridApiContext } from '@mui/x-data-grid';
import DeleteIcon from '@mui/icons-material/Delete';
import { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'
import Loading from '../../../Components/Loading.component';
import { UserContext } from '../../../Context/User/UserContext.context';
import Movie from '../../Domain/Domain-Model/Entities/Movie.model';
import MovieRepository from '../../Domain/Domain-Service/Repository/Implementation/movie.repository.service';
import Genre from '../../Domain/Domain-Model/Entities/Genre.model';
import { randomId } from '@mui/x-data-grid-generator'

const useChangeValidation = () => {
return useCallback(
(movie: Partial) =>
new Promise<Partial>((resolve, reject) =>
setTimeout(() => {
console.log(" useChangeValidation")
/* if (movie.title === '') reject(new Error(Error saving ${movie.idMovie}, it can not be empty))
else resolve({ ...movie, title: movie.title });
if (movie.director === '') reject(new Error(Error saving ${movie.idMovie}, it can not be empty))
else resolve({ ...movie, director: movie.director });*/
resolve({ ...movie, director: movie.director })
}, 0),
),
[],
);
};

export const MovieTable = (props: any) => {
const [columns, setColumns] = useState<GridColumns>([]);
const [rows, setRows] = useState<any[]>([])
const [movie_repository] = useState(new MovieRepository());
const [user] = useContext(UserContext)
const mutateRow = useChangeValidation();
const [snackbar, setSnackbar] = useState<Pick<AlertProps, 'children' | 'severity'> | null>(null);
const handleCloseSnackbar = () => setSnackbar(null);
const [selectionModel, setSelectionModel] = useState([]);
const rowsRef = useRef(rows);

useEffect(
    () => {
        rowsRef.current = rows;
    },
    [rows],
);

const processRowUpdate = useCallback(
    async (row: GridRowModel) => {
        console.log("processRowUpdate")
        let apiCall = await movie_repository.putMovie(new Movie(row.id, row.title, row.releaseDate, row.director,
            new Genre((await movie_repository.getMovieById(row.id, user.token)).genre.idGenre as number, row.genero)), user.token)
        const response = await mutateRow(row);
        if (apiCall !== undefined) setSnackbar({ children: `Movie ${apiCall.title} successfully updated`, severity: 'success' });
        return response;
    },
    [mutateRow],
);

const handleProcessRowUpdateError = useCallback((error: Error) => {
    console.log("handleProcessRowUpdateError")
    setSnackbar({ children: error.message, severity: 'error' });
}, []);


const handleAddRow = () => {
    setRows((prevRows) => [{
        id: randomId(),
        title: '',
        releaseDate: '',
        director: '',
        genero: '',
    }, ...prevRows]);
};

async function handleDeleteRow(params: any) {
    let response = undefined;
    response = await movie_repository.deleteMovieById(params.id, user.token)
    if (response !== undefined) {
        setRows(rowsRef.current.filter(row => row.id !== params.id))
    }
}
useMemo(() => setColumns([
    { field: 'id', headerName: 'Id', width: 90, },
    { field: 'title', headerName: 'Título', width: 160, editable: true },
    { field: 'releaseDate', headerName: 'Fecha', type: 'dateTime', width: 220, editable: true },
    { field: 'director', headerName: 'Director', width: 160, editable: true },
    { field: 'genero', headerName: 'Género', width: 130, editable: true },
    {
        field: 'action', headerName: 'Action', width: 90, renderCell: (cellValues: any) => {
            return (
                <IconButton aria-label="delete" size="large" onClick={(event) => { handleDeleteRow(cellValues); }}>
                    <DeleteIcon fontSize="inherit" />
                </IconButton>
            );
        }
    }
]), [])

useMemo(() => {
    let response: Movie[];
    (async () => {
        console.log("getAllMovies")
        response = await movie_repository.getAllMovies(user.token)
        if (response !== undefined) {
            setRows(response.map((row) => ({
                id: row.idMovie,
                title: row.title,
                releaseDate: row.releaseDate,
                director: row.director,
                genero: row.genre.type
            })));
        }
    })()
}, [])


return (
    <>
        <div style={{
            height: '56vh', width: '60vw', marginLeft: '22%',
            marginTop: '5%'
        }}>
            <Box sx={{ width: '100%' }}>
                <Box sx={{ height: 400, mt: 1 }}>
                    {rows.length > 0 ?
                        <>
                            <DataGrid
                                rows={rows}
                                columns={columns}
                                editMode="row"
                                onCellEditStop={(params: GridCellEditStopParams, event: MuiEvent) => {
                                    if (params.reason === GridCellEditStopReasons.cellFocusOut) {
                                        event.defaultMuiPrevented = true;
                                        alert('sto]ed')
                                    }
                                }}
                                //getRowId={(row) => row.id}
                                processRowUpdate={processRowUpdate}
                                onProcessRowUpdateError={handleProcessRowUpdateError}
                                components={{ Toolbar: GridToolbar }}
                                experimentalFeatures={{ newEditingApi: true, warnIfFocusStateIsNotSynced: true, preventCommitWhileValidating: true }}
                                onSelectionModelChange={(newSelectionModel: any) => {
                                    setSelectionModel(newSelectionModel);
                                }}
                                selectionModel={selectionModel}
                                // isRowSelectable={(params: GridRowParams) => params.row.action}
                                checkboxSelection />
                            <Stack direction="row" spacing={1} sx={{ marginLeft: '54vw' }}>
                                <Button size="small" onClick={handleAddRow}>
                                    Add a row
                                </Button>
                            </Stack></> :
                        <Loading />
                    }
                    {!!snackbar && (
                        <Snackbar
                            open
                            onClose={handleCloseSnackbar}
                            anchorOrigin={{ vertical: 'bottom', horizontal: 'center' }}
                            autoHideDuration={3000}>
                            <Alert {...snackbar} onClose={handleCloseSnackbar} />
                        </Snackbar>
                    )}
                </Box>
            </Box>

        </div>
    </>
)

}

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

No response

@tonycerdas tonycerdas added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 21, 2022
@alexfauquette
Copy link
Member

Could you provide a minimal reproduction example?

I tried to run your code
https://codesandbox.io/s/customcolumnmenu-demo-mui-x-forked-gqdktf?file=/demo.tsx:472-851

But of course nobody can knows what is behind those lines except you

import Loading from "../../../Components/Loading.component";
import { UserContext } from "../../../Context/User/UserContext.context";
import Movie from "../../Domain/Domain-Model/Entities/Movie.model";
import MovieRepository from "../../Domain/Domain-Service/Repository/Implementation/movie.repository.service";
import Genre from "../../Domain/Domain-Model/Entities/Genre.model";

@alexfauquette alexfauquette added component: data grid This is the name of the generic UI component, not the React module! status: waiting for author Issue with insufficient information feature: Editing Related to the data grid Editing feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 22, 2022
@tonycerdas
Copy link
Author

tonycerdas commented Jun 22, 2022

https://drive.google.com/file/d/19VhT6-CgfbdRVb1Hg4rqBWXSh_hCqSZb/view

Take a look on this video, I was Trying to write: Hola cómo estás, but it got some lag and deleted some words
It happen when this is true: experimentalFeatures={{ newEditingApi: true}}
If I put it false that behavior does not happen

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Jun 22, 2022
@alexfauquette
Copy link
Member

I do not doubt you are experiencing lags, but without a reproduction, we can not know which feature slows down the component, and so we are not able to fix it

It's probably not the newEditingApi alone or the number of rows, because it runs without problems in the following codesandbox with 500 rows

https://codesandbox.io/s/basiceditinggrid-demo-mui-x-forked-1my45w?file=/demo.tsx

Could you provide the version of the package you are using to verify we did not add some perf improvement between your version and the last one

Maybe @m4theushw has already seen this problem in another issue

@TheRealCuran
Copy link

Some of my users report this issue with editing of simple text fields too. The tables have all about 60-100 rows and 12 columns. Most columns render simple text or links (some with <Tooltip>), one column renders a picture and a <IconButton> through which you can upload a new picture. The remaining two render a <Button> (showing a state; clicking it triggers the edit state and opens the select) and an image, which again is showing a sort of a state and can be clicked to open a select. Both are native <DataGrid>-selects and the text fields are also using <DataGrid> own text edit fields. The only exception to this is the picture upload which is handled through an external dialog and not the edit mode directly.

This means I have a total of two text columns and select columns, that can be edited per row. All of those have a simple valueSetter attached and I did notice, that I seem to enter that on each keystroke. The function looks like this:

valueSetter: (
  params: GridValueSetterParams<MyGridRowData, string>,
) => {
  // my_field is only there, because it is easier to access in DataGrid
  // it's proper place is in sub_element
  const my_field = params.value
  let sub_element: TeaserInfo | undefined
  if (params.row.sub_element) {
    sub_element = params.row.sub_element
    sub_element.my_field = my_field
  }
  return { ...params.row, my_field, teaser }
},

@TheRealCuran
Copy link

Some more testing yielded: if I set debounceMs to 500 in @mui/x-data-grid/components/cell/GridEditInputCell.js (the version of GridEditInputCell I'm using) everything works as it should (most of the time, see below).

The problem always starts, when you hit the debounceMs value, ie. pause "too long" in your input. Whatever is run then takes too long and, also resets the cursor/input to the content that was processed, making everything you might have put in afterwards disappear. When you delete characters the cursor position usually gets horribly mangled.

Maybe the solution would be to only trigger apiRef.current.setEditCellValue() at the very end, ie. when the content change is to be committed?

@m4theushw
Copy link
Member

@TheRealCuran could you provide a reproduction? I can't know if you're using cell editing or row editing.

@m4theushw m4theushw added the status: waiting for author Issue with insufficient information label Jun 30, 2022
@TheRealCuran
Copy link

@m4theushw I use only cell editing (users double click into a cell and can change its value). The reproducer will take me until tomorrow at least, might become next week. Need to sanitize the internal code in a way that I can share and still shows the issue.

Apart from that, I can upgrade "some of my users" to "all users" of this internal tool. All of them are using the latest Firefox on Mac OS; but I do see this issue on Linux (latest Firefox) as well. Just to make sure this is now bug in Firefox, I did test Chromium as well and saw the same issue.

@tonycerdas
Copy link
Author

tonycerdas commented Jun 30, 2022

I "fixed" it by removing the ExperimentalApi and using onRowEditStop instead. I didn't know how to solve it without removing that. Though. I've also added a SearchaBar to the GridToolBar and it also has typing issues. I have noticed that it has some delay when the field checks the spelling.

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Jun 30, 2022
@oliviertassinari oliviertassinari added the status: waiting for author Issue with insufficient information label Jul 8, 2022
@oliviertassinari oliviertassinari changed the title Lag while typing in datagrid component [data grid] Lag while typing Jul 8, 2022
@colespencer1453
Copy link

colespencer1453 commented Jul 8, 2022

@alexfauquette @m4theushw I am experiencing a similar issue that I think may be a repro of what the OP is talking about. Randomly while editing a cell a character will get deleted. It happens so quickly it is hard to tell if it actually happened, but I was able to capture it in a screen recording.

Repro: https://codesandbox.io/s/quizzical-sky-ez5zql?file=/src/App.js

During second 2 of the video below if you scrub slowly you can see a Q is typed and then quickly erased although the backspace button was not pressed. Watching in real time you can hardly tell it was typed at all.

Screen.Recording.2022-07-08.at.6.19.54.PM.mov

@TheRealCuran
Copy link

I solved the issue for myself. Firefox Profiler to the rescue: I had an Ajv validator function that got in the way. I mean it schouldn't have been executed anyway, but since we go through the valueSetter, that apparently had the side effect of causing a recompilation of the Ajv validator function (why I haven't investigated). Anyway, the solution for me was to pre-compile my Ajv and not just have them included from a central cache.

@bimbiltu
Copy link

I've noticed that this issue appears pretty often when running my app in debug mode, however it is very rare in prod builds. Unfortunately I do not have time to attempt to create a reproducer right this moment ☹️ From this and other posts in the thread, like the one from @TheRealCuran above, it seems like the frequency of this issue popping up is tied to the general performance of the page the grid is on

@colespencer1453
Copy link

@bimbiltu The repro I posted above has nothing to with any components other than the grid. Not sure if thats what you were saying, but this is happening in isolation of the grid.

@bimbiltu
Copy link

@colespencer1453 I spent a while playing with the link you posted and got the same issue to reproduce, but its very infrequent for me. I have to fill in the cell maybe 20-30 times very quickly to get it to happen. I was hoping to come up with a more reliable reproducer (its very easy to repro inside my app), but what you linked does show the bug for me as well

@m4theushw m4theushw removed the status: waiting for author Issue with insufficient information label Jul 14, 2022
@colespencer1453
Copy link

@m4theushw I'd love to bump this issue. I'm seeing this pretty consistently and it is super annoying. Has anybody on the mui-x team not observed this behavior?

@m4theushw
Copy link
Member

@colespencer1453 I managed to reproduce it by increasing CPU throttling in Chrome DevTools. But I'm still investigating it.

@m4theushw
Copy link
Member

I opened #5646 with a potential fix for this bug. Could anyone check if it solves the problem? The docs for the PR is https://deploy-preview-5646--material-ui-x.netlify.app/x/react-data-grid/editing/. Opening any demo in CodeSandbox will automatically use the package with the fix. You can also use this package in your application. It's very hard to reproduce this bug, even using CPU throating, so I had to force a delay to test locally.

@bimbiltu
Copy link

bimbiltu commented Aug 1, 2022

@m4theushw that PR appears to solve the issue for me

@park-siwan
Copy link

park-siwan commented Aug 12, 2022

Corrected to "react": "^17.0.2", "react-dom": "^17.0.2" in package.json and npm install to resolve the input delay.
like this #5729, I checked that it works on React 17 and not on 18.

@cherniavskii
Copy link
Member

cherniavskii commented Aug 17, 2022

I think I've experienced same behaviour as described in #5283 (comment)
Sometimes letters might get "swallowed" during editing:

Screen.Recording.2022-08-17.at.18.13.03.mov

You can slow down the video to make sure that I type "hello" each time by looking at the virtual keyboard.
2 out of 3 times a letter was "swallowed".

I've managed to create an e2e test to reproduce the issue:

diff --git a/test/e2e/fixtures/DataGrid/GridEditing.tsx b/test/e2e/fixtures/DataGrid/GridEditing.tsx
new file mode 100644
index 0000000..63ef0d2
--- /dev/null
+++ b/test/e2e/fixtures/DataGrid/GridEditing.tsx
@@ -0,0 +1,19 @@
+import * as React from 'react';
+import { DataGrid, GridColumns, GridRowsProp } from '@mui/x-data-grid';
+
+const columns: GridColumns = [{ field: 'name', headerName: 'Name', width: 600, editable: true }];
+
+const rows: GridRowsProp = [
+  {
+    id: 1,
+    name: 'Name',
+  },
+];
+
+export default function BasicEditingGrid() {
+  return (
+    <div style={{ height: 300, width: '100%' }}>
+      <DataGrid rows={rows} columns={columns} experimentalFeatures={{ newEditingApi: true }} />
+    </div>
+  );
+}
diff --git a/test/e2e/index.test.ts b/test/e2e/index.test.ts
index a7aceec..0ad431a 100644
--- a/test/e2e/index.test.ts
+++ b/test/e2e/index.test.ts
@@ -350,5 +350,30 @@ describe('e2e', () => {
         await page.evaluate(() => (document.activeElement as HTMLInputElement).value),
       ).to.equal('0-1');
     });
+
+    it.only('should not swallow letters during cell editing', async () => {
+      await renderFixture('DataGrid/GridEditing');
+
+      await page.click('[role="cell"][data-field="name"]');
+
+      await page.keyboard.down('Backspace');
+      await sleep(300);
+
+      await page.keyboard.down('Digit1');
+      await sleep(30);
+
+      await page.keyboard.down('Digit0');
+      await sleep(210);
+
+      await page.keyboard.down('Digit1');
+      await sleep(35);
+
+      await page.keyboard.down('Digit0');
+      await sleep(300);
+
+      expect(
+        await page.evaluate(() => (document.activeElement as HTMLInputElement).value),
+      ).to.equal('1010');
+    });
   });
 });

Few takeaways:

  • it's related to debounce
  • the key line in the above is await sleep(210);. Changing it to 150 or 250 would make test pass. On my machine it consistently fails with values ~205..~210
  • the issue is only reproducible with new editing API

I'll check if #5646 makes the test pass.

UPDATE: the test above fails starting from this commit c049e3a. It passes on the commit before (17a2570) and I cannot reproduce the issue manually too.
So it looks like the issue exists only when using React 18 and new editing API

UPDATE 2: the test seems to pass on #5646 rebased onto master

@TheRealCuran
Copy link

[Preface: I am no longer affected directly by this issue, since I've found a workaround. Feel free to ignore my input below.]

@bimbiltu writes:

I've noticed that this issue appears pretty often when running my app in debug mode, however it is very rare in prod builds. Unfortunately I do not have time to attempt to create a reproducer right this moment frowning_face From this and other posts in the thread, like the one from @TheRealCuran above, it seems like the frequency of this issue popping up is tied to the general performance of the page the grid is on

If you take that away from my post, you didn't read it properly, IMHO. The page itself is fine and the Ajv validation should only happen on network traffic and even then there should never have been a recompilation of the validation function, but for whatever reason (again, I haven't gone done that rabbit hole, since there was an easier solution for relatively modern hardware – not optimal by any means, since I am sure this is still somehow going through validation/Ajv when it shouldn't) it triggered a recompilation of the Ajv validation function.

Having the valueSetter called on more or less each keystroke has the side effect of dropping values entered, if whatever valueSetter (and maybe other parts/callbacks that get called here) is "too slow" for the next key press. In my personal opinion the underlying issue is not to go through all those call backs on each callbacks (maybe except some validation, if that is defined in the column definition) on each key stroke in edit mode. Only call those on "end of edit mode". And if that is impossible for whatever reason: build a queue of key strokes, but don't "reset" the value to some imperfect state.

That being said: by having a static Ajv function, compiled during commit, that is only annoying myself, I managed to get below the threshold of the typing speed my users have. Therefore I cannot reproduce this issue in production any longer. Still, going through large parts of the callback chain seems excessive for each keystroke.

If the PR is reducing the amount of callback executions to the bare minimum, it is certainly going to help. Anyway, feel free to merge, I didn't see any reduction in performance for the users here.

@m4theushw
Copy link
Member

@cherniavskii I tried your test case. It failed on master on my machine with await sleep(205) but didn't fail when running with the Playwright image via docker. I didn't add it to #5646 because it doesn't seem very reliable: we may have in the end a test that passes even without the fix.

@m4theushw
Copy link
Member

Fixed by #5646

@ithrforu
Copy link
Contributor

ithrforu commented Nov 7, 2022

Hello, @m4theushw! Is it fixed in DataGridPremium? I have same problem with quick filter with and without debouncer. It's rarely reproduced in production and more often in dev build.

@m4theushw
Copy link
Member

@ithrforu The fix is also available in the latest version of DataGridPremium. Could you create another issue with a video showing the bug, and a reproduction if possible? The fix from #5646 is only for the editing API, the quick filter may need a similar solution.

@ithrforu
Copy link
Contributor

ithrforu commented Nov 8, 2022

@m4theushw, thank you for your attention! 6783

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: Editing Related to the data grid Editing feature performance
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.