Really slow React performance with category + nested array with many items

[georgiosd]

The object itself is not that complex, but it has a nested array with about 100 short string:

{
 prop1: ...,
 prop2: ...,
 dict: {
  A: [...array of strings],
  B: [...array of strings],
  C: [...array of strings]
 }
}

Changing one of the strings value causes a ton of re-renders in, presumably, the array editor:

This is on latest stable: 2.4.0 with material-ui renderers

[georgiosd]

EDIT: the issue persists in 2.4.1-beta.0

[sdirix]

Hi @georgiosd, thanks for the report. Looking at your screenshot it seems like we construct new callback functions, e.g. moveUp and moveDown for each row in each render call. Because of this our default React.memo is failing to cache the result for the unchanged rows and they are rerendered. Checking the code confirms this suspicion:

jsonforms/packages/material/src/complex/MaterialTableControl.tsx

Lines 361 to 362 in 83a4dd2

	moveUp={moveUp(path, index)}
	moveDown={moveDown(path, index)}

To fix this we either need to customize the props check in React.memo or make sure to only construct these functions when necessary, i.e. use useCallback. In general I would prefer the latter approach.

[georgiosd]

@sdirix that would make sense to me also - if you promise to publish a new beta after the fix, I can send a pull request :)

[sdirix]

If the PR meets our standards, is relatively small, straightforward to understand and fixes this issue I would be willing to add it to this release (i.e. publish a new beta). If the PR itself is fine but it turned out that more things had to be changed and therefore the PR feels "risky", then I would add it to the first alpha of the next release instead.

So I won't promise you a new beta. It depends purely on the PR and whether we correctly identified the issue whether we can make it part of the release so late in the cycle (release is this month) or not. In any way, if the PR is fine it will be part of some released JSON Forms pretty soon (beta this release or alpha next release).

[georgiosd]

@sdirix of course the PR will have to pass your "tests" also, I wasn't expecting a free ride so to speak.

I asked for that because I'm about to deliver a new project and this is gonna cause some complaints from the client so a hotfix release would be appreciated.

Do you know if there is an easy way to keep the project locally and load it in my project instead of the npm registry copy?

[sdirix]

I can recommend yalc. That's what we use to test packages locally, for example to test new JSON Forms versions in the seed repository.

[georgiosd]

Thank you @sdirix .

I'm also seeing another problem... I've created a custom control which I'm successfully matching to multiple locations.

What I'm seeing is that the memo will block a re-render of the control when it's rendered for another scope.

Any pointers?

[sdirix]

Hmm I'm not sure whether the memo is responsible for this as each rendered control is an own instance. The memo just avoids rerendering a specific instance when none of its props changed and doesn't sync multiple instances. You can easily see this with the provided renderer set as for example all text inputs will match to the same renderer, i.e. the same renderer also matches on multiple controls.

I would have to look at your custom renderer to determine what the problem is.

[georgiosd]

I think I fixed this one, thank you.

I will see if I can improve on the rendering and let you know

[sdirix]

Closing this for now. Please reopen when you have further problems :)