Deep fix for freezing of table

[Domino987]

Related Issue

Fixes #2486, Fixes #2453, Fixes #2451, Fixes #2404,

Description

This PR removes the while loop that caused the infinity loop in 1.69.0 and replaces it with a regex call to exchange the percentage value with the pixel value reducePercentsInCalc .
calc((100% - (0px)) / 9) to calc((1054px - (0px)) / 9)
or calc(calc((100% - (0px)) / 9) + -26px) to calc(calc((1054px - (0px)) / 9) + -26px).

[iamsaurabhgupt]

@mbrn we are also facing this freezing of table in production.
Please have a look at this PR and merge it.

[Domino987]

@mbrn we are also facing this freezing of table in production.
Please have a look at this PR and merge it.

Hi I wrote a quick fix and mbrn foinf the time to release this with 1.69.1. this should stop the freezing. This is just the follow up to correctly fix it instead of the hot fix. He will be back in November.

[iamsaurabhgupt]

@Domino987 what's the hotfix here? we downgraded to 1.67.0 but still facing issues on bulk edit.
After changing 2-3 items, the whole screen freezes on submission (tick mark)
How to solve it?

[Domino987]

@iamsaurabhgupt, The hotfix was for the freezing during a rerender introduced by the whole loop of the cell width dragging. The bulk edit freezing may be related to something else. Can you reproduce that with the newest version 1.69.1 and link a Sandbox?

[iamsaurabhgupt]

@Domino987 please check this sandbox https://stackblitz.com/edit/material-table-bulk-edit-atss27?file=package.json
Try to bulk edit 10 rows and make 10-15 changes and then try to save it. It will get hanged.

[Domino987]

@iamsaurabhgupt I looked at your code and it seems that its more a problem with how you set the next state, instead of a problem with the table. For each change, you loop through all the data and merge them and use the hook each time. This is a quite expensive operation so I propose to use this code to reduce the setState amounts and loopings:

if (changes) {
    setState(prevState => ({
        ...prevState,
        data: prevState.data.map(d => changes[d.tableData.id]?.newData ?? d)
     }));
}

instead of

onBulkUpdate: changes =>
          new Promise((resolve, reject) => {
            setTimeout(() => {
                                resolve();
                                if (changes) {
                                    let newData = [];
                                    Object.keys(changes).map((editData) => {
                                        setState((prevState) => {
                                            const data = [...prevState.data];
                                            data[data.indexOf(changes[editData]['oldData'])] =
                                                changes[editData]['newData'];
                                            newData = [...newData, changes[editData]['newData']];
                                            return { ...prevState, data };
                                        });
                                    });
                                }
                            }, 1000);
          }),   

[iamsaurabhgupt]

@Domino987 You're right, it was our code. Thanks a lot for saving the day. 💯

[iamsaurabhgupt]

@Domino987 we are still facing performance issues on the table v1.91.1 even after implementing your code.
After 10-15 edits, the saving gets incrementally slower.
@mbrn can you please merge this PR? I hope this might solve it.

[Domino987]

@iamsaurabhgupt I do not know when this will be merged. Can you verify that this fixes it for you. I could not reproduce it with this fix bit maybe you find a different edge case.

[masbaehr]

Does not fix the problem for me too unfortunately

[Domino987]

@masbaehr what do you experience exactly?

[faramos]

@Domino987
Do you have a workaround that you can suggest while we wait for the review?
Thanks in advance.

[rvit23]

I'm facing the same issue while adding more than 5 entries continuously. Is there any fix?

[Domino987]

Until this is merged only going down to 1.68 fixes this

[lbuscherX]

Do we have an idea on when this will be merged?

[Domino987]

I talked to mbrn 2 days ago and he will be taking a look soon. But I do not have more info.

[lbuscherX]

Thanks Domino.

To be clear on my end (and hopefully contribute to our collective understanding), my table-freezing seems to be caused when a user tries to edit multiple lines in a table, one after the other. As others have described, the first instance or two may work, but with each edit, the behavior of the page slows and eventually freezes.

The below code is what I believe is the cause (since it's what's triggered with each edit):

editable={{
                    onRowUpdate: (newData, oldData) =>
                    new Promise((resolve, reject) => {
                        const updateOrphanActions = async () => {
                           await axios.post(
                              //removed post contents for security - not relevant to problem or error description
                            )};
                        updateOrphanActions();
                        const dataUpdate = [...data];
                        const index = oldData.tableData.id;
                        dataUpdate[index] = newData;
                        setData([...dataUpdate]);
                        resolve();     
                    }),
            }}

Domino, I saw above that you mentioned that this approach to updating data could be fraught with performance issues. Would you advise that I also make the changes you suggested above (or some other change)? Or would it have little effect until the deep fix is implemented...

edit: fixed codeblock format insertion into comment ^

[chattling]

I don't think updating the data in our code is the reason for freezing. I'm using Apollo useMutation to update data and getting it back from Apollo cache from useQuery. So I don't have any logic to update state besides what is done by Apollo. And for me it behaves the same, it is freezing after multiple updates, and I don't have bulkUpdate implemented, updates only row by row:

This is how I set up query and mutation and retrieve data set from data returned from Apollo useQuery.
For those unfamiliar with Apollo - After 1st retrieve by useQuery Apollo caching all records. And after useMutation called (via updateDDTask), returned object updated in Apollo cache and useQuery cause re-render returning updated data:

const { loading, error, data } = useQuery(DD_TASKS_Q, {
    variables: { ddActivityId: DDActivity.id },
  })

const [updateDDTask] = useMutation(UPDATE_DD_TASK_M)

const { DDTasks } = data

and material-table editable setup like this:

editable={{
        onRowUpdate: (newData, oldData) => updateDDTask({ variables: { ...newData } }),
      }}

[Domino987]

@lbuscherX I agree with @chattling that rerenders in general and not the edits themselves are the problem, due to the while loops that I am removing in the PR.
Can you pull the code of this PR locally and verify, that it works for you with your code?
That would be a great help to verify this.

[chattling]

Can you pull the code of this PR locally and verify, that it works for you with your code?

Hmm, interesting thing happened. I tested with 1.69.1 like 2 weeks ago and could reproduce. So I switched back to 1.67.1.
Today, since you asked, before I took your PR I wanted to reproduce first. So I switched to 1.69.1 (without your PR) again and now I cannot reproduce. I did so many edits, I think it works for me even without your PR. I'll play with it more..

[chattling]

I reverted the npm packages to time when I could reproduce and upgraded material-table to 1.69.1 and could reproduce on second edit. It froze and after 30 seconds re-rendered but screen was unresponsive and then after another 30 seconds got JavaScript error (see below). I'll try to upgrade all the npm packages again and re-test and will let you know which npm packages I upgraded if it is no longer reproducible.

[chattling]

Ok, so I upgraded npm packages back and can no longer reproduce the issue. These are the upgrades I did that I think relevant (I have more packages upgraded, but don't think they being used for it):
react/react-dom: 16.13.1 -> 17.0.1
@apollo/client: 3.1.4 -> 3.2.5

material-table remained 1.69.1 between tests

[toinhao1]

Any idea when this will be merged in?

[ewanmellor]

The WhileLoopRemoval changeset in this PR removes the calculation for the percentage that it's replacing. So if it is replacing "100%" then it is doing the right thing, but for any other value it is now substituting the wrong width.

[Domino987]

@ewanmellor So if not 100% is set, it should calculate the difference of those values like fullValue * number to be replaced?

[Domino987]

It now respects not 100% values

[mbrn]

Thanks.

[stephenjtyler2]

So I don't think those fixes will address the problem I saw when I was digging into this. Here is what I saw:

reducePercentsInCalc was getting called for every cell and was ultimately causing the cpu spin/freeze but the problem was not in the logic of that function per se.

I have a large table of 100 rows of like 14/15 columns and net it was getting called 1920 times per render. But that was not a problem when the "calc" expression contained 1 'n%' as it did on the first render. However on the second render that calc expressions had ballooned up to some awful long nested calc statement that actually contained 16 'n%' occurences. So 16 * as much work for each of the 1920 calls. The third render had 32. The fourth render - the browser froze.

I believe the real issue here is not the behaviour reducePercentsInCalc per se, or the act of doing column recalculation or not based on the presence or absence of a set width in the columndef. Hence, I believe the "fix" checked in here is at best a workaround. The first change replaed the logic in reducePercentsInCalc with a regex which as other posters pointed now does the wrong thing because it replaces with the full width value rather than the calculated value. And then the other 'fix' makes it so it doesn't even call this at all if you have set a width on the columnDef. So basically a workaround on top of a bug that was trying to fix a problem by treating a symptom.

And even with the previous "count" limit to the while loop that limited the loop in reducePercentsInCalc, the blowing up of the css calc expression itself from render to redner caused a meltdown in the browser regardless of keeping reducePercenttsInCalc under some degree of control. i.e. that function is NOT the problem.

I suspect the amount of blow up in the expression (1->16->32 %s in my case - which is like 50->350->700 chars of calc expression) may be related to number of columns in the table, so it may have been particularly bad for me because of the combination of the number of columns (making the calc expression grow very fast render to render) and the sheer number cells causing the handling of that to be called 1920 times per render.

So... I think real 'fix' needed is to sort out the logic problem in the the way that column widths are calculated (when they are) so that they don't cause that massive expansion of the calc expression for each column. And then revert this workaround.

For now setting width: "auto" on all the column defs, also sidesteps the issue (with 1.69.1) so it's not drop dead urgent for me but I may try to dig in and see if I can do and contribute the real fix. But I thought I'd share what I've learned about this anyway in case someone else wants to take a deeper look at this based on this info.

[Domino987]

@stephenjtyler2 Thank you for the detailed explanation. I agree that the root cause must be found, but I do not experience the inflation of the css width anymore after this merge. Can you verify, that this is still a problem and maybe create a sandbox if it still persists?

[ar4hc]

I have seen a large css prop like described by @stephenjtyler2 too. And, of course, not really reproducible, so only anectdata...

Still i think the root cause is some css value strings are growing very large very fast in some (tm) cases, too.
And the regex is just fast enough to deal with that...

Will watch out for this behavior and try to add valid data to this ticket.

[peacechen]

See #2689 for a fix that may be the same root cause

[ShoshanaTzi]

I also get this problem,
When uses columns with hooks like this, after 2-3 setData(updateData) the screen get freezing
const [columns, setColumn] = React.useState([ { title: 'Avatar', field: 'imageUrl', render: rowData => <img src={rowData.imageUrl} style={{width: 40, borderRadius: '50%'}}/> }, { title: 'Name', field: 'name' }, { title: 'Surname', field: 'surname' }, { title: 'Birth Year', field: 'birthYear', type: 'numeric' }, { title: 'Birth Place', field: 'birthCity', lookup: { 34: 'İstanbul', 63: 'Şanlıurfa' }, }, ]);

But when remove the hooks, and use simple consts columns, it's working well

[Domino987]

Its due to the mutation of the data and columns. To change it, switch to material-table-core.