Devtools are re-rerendering constantly on every cache update even when closed

[pristas-peter]

Describe the bug

We are encountering performance issues with the app when using devtools with many queries (around 1000). We are using normalised cache keys, when for example fetch of list items updates data in each single fetch query via setQueryData.

However in v4, by using useSyncExternalStore things got really slow (app freezes for a short time when doing updates).

Is it possible to update devtools by batches, or is it even necessary to "observe" every query when the devtools are closed?

Your minimal, reproducible example

Steps to reproduce

Expected behavior

How often does this bug happen?

No response

Screenshots or Videos

No response

Platform

react-query version

v4 3.2.1

TypeScript version

4.7.4

Additional context

No response

[TkDodo]

Please provide a codesandbox reproduction! By using useSyncExternalStore, we actually fixed a perf issue in the devtools:

• https://github.com/TanStack/query/issues/2742#issuecomment-993923506

[pristas-peter]

Hi, check it out here: https://codesandbox.io/s/suspicious-jasper-xh7k88?file=/src/App.js

Try to scroll beyond 500 items or so, the app will become un-responsible. If you comment out react query devtools component, all is smooth.

This was not happening in react-query 3.

Thanks for checking it out.

[TkDodo]

i can confirm that this is not how it should be :)

Given that it gets worse with the number of queries, my best guess is that all uSES subscriptions are being triggered to re-render even though they should not. We've switched to fine-grained subscriptions because they are supposed to be more performant. Before that, every update would re-render the whole devtools, which is why we had the workaround of not subscribing at all when the devtools are closed.

I see a couple of ways forward:

1. re-add the workaround that we removed in #2742. Basically, render null when the devtools are closed. That would be a quick win, and probably not wrong because why would we subscribe to anything if we are closed. However, app would still lag with open devtools.
2. investigate what re-renders so often and why, and stop those re-renders.
3. if the renders are not coming from uSES subscriptions themselves but from top-down renders, we could try memoizing each query row. But I don't think that's the case.

I'm afraid I won't have much time to look into this right now. Would you like to contribute any of those steps @pristas-peter ?

[pristas-peter]

Yes, I will try to take a look at it next week

[dhrjarun]

i can confirm that this is not how it should be :)

Given that it gets worse with the number of queries, my best guess is that all uSES subscriptions are being triggered to re-render even though they should not. We've switched to fine-grained subscriptions because they are supposed to be more performant. Before that, every update would re-render the whole devtools, which is why we had the workaround of not subscribing at all when the devtools are closed.

I see a couple of ways forward:

1. re-add the workaround that we removed in speedup devtools #2742. Basically, render null when the devtools are closed. That would be a quick win, and probably not wrong because why would we subscribe to anything if we are closed. However, app would still lag with open devtools.
2. investigate what re-renders so often and why, and stop those re-renders.
3. if the renders are not coming from uSES subscriptions themselves but from top-down renders, we could try memoizing each query row. But I don't think that's the case.

I'm afraid I won't have much time to look into this right now. Would you like to contribute any of those steps @pristas-peter ?

Hey @TkDodo, Can I work on this?

[TkDodo]

I am not working on it and I don't know if @pristas-peter is doing anything, so yeah, go ahead 🚀

[dhrjarun]

cool

[pristas-peter]

Sorry guys, I was little busy this week as well. @DhrjArun if you find out something, let us know 💯

[dhrjarun]

sure @pristas-peter

[dhrjarun]

Not rendering DevToolPanel if closed solves the issue temporary.

I tried adding React.memo to QueryRow and DevToolPanel Components, but it didn't fix the issue.

re-renders are coming from uSES subscriptions, and it’s being called even if theres no change in the queryCache

In this case, on each next fetch, app gets 100 new items, thats why after 4 - 5 fetch app becomes unresponsive. I think the issue can be solve by virtualizing the list(QueryRows).

[dhrjarun]

Hi @TkDodo what should I do?

[TkDodo]

Not rendering DevToolPanel if closed solves the issue temporary.

I think this is something we should do in any case. There is no point in keeping subscriptions when they are closed, and whatever future issues might pop up, users can always work around it by closing the devtools.

re-renders are coming from uSES subscriptions, and it’s being called even if theres no change in the queryCache

It would be good to find out which subscription has the problem, as we have multiple, fine-grained subscriptions:

https://github.com/TanStack/query/blob/ea80700c2c8d48c6d0cb4ae0165a424ebea5d35c/packages/react-query-devtools/src/devtools.tsx#L706-L731

Theoretically, as long as the result returned from the selectors doesn't change, it shouldn't trigger a re-render. But maybe it's the activeQueryState one?

@prateek3255 fyi

[TkDodo]

I've merged the PR with the quick fix. However, I think it's still a problem when the devtools are opened that we should look into.

[prateek3255]

Will check out what's wrong with the uSES subscription 👍

[dhrjarun]

I think even if you remove all the uSES subscriptions except the first one. It will cause the same problem with 400-500 queryRows.

[TkDodo]

any updates / progress on this issue @prateek3255 ?

[prateek3255]

@TkDodo Was facing issues in local dev with linking devtools with one of the examples initially, but have got that figured out now. I will investigate this and share an update by today or tomorrow

[prateek3255]

@TkDodo So I ran React Profiler on the example above, and I think the culprit is not uSES here. The individual QueryRow's take significant time to render, since in the above example 100 queries are added to the cache in one go.

One thing I also noticed was that whenever new query rows were added all the query rows are re-rendered right now since the parent component re-renders. So I memoized the QueryRow component and saw some improvement in the render durations.

Here this is what the profiler looks when I load 400 rows with the way it is right now:

https://user-images.githubusercontent.com/21277179/198930719-4f04bc4d-f000-4f51-8094-93b1f2b8a39d.mov

In the above video notice with every commit (whenever new 100 rows are added), all the QueryRows are re-rendered, and the render time on the right increases with every commit significantly on the right.

Now this how the profiler looks like when I memoized the QueryRow component:

Screen Recording 2022-10-31 at 9.46.56 AM.webm

If you look closely you'll see the QueryRow components that have already been rendered are greyed out and only the new QueryRow components are rendered, also the render durations on the right are also less than what they were without memo. (For some reason the new QueryRow components take more time to render than the previous one which I am not sure why).

All though even with memo, I see improvement in the performance of the app, but it still isn't significant enough. I guess we should add the memo and keep rendering null when devtools is closed. What do you think?

[TkDodo]

I guess we should add the memo and keep rendering null when devtools is closed. What do you think?

Absolutely 💯

[prateek3255]

@TkDodo opened a PR with #4416