Add a staleTime to queries #1167
Conversation
|
Your Render PR Server URL is https://toolpad-pr-1167.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cd6jqhmn6mpqe70nm1jg. |
oliviertassinari
requested a deployment
to
staleTime - toolpad-db PR #1167
Render
Abandoned
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
| @@ -872,6 +878,7 @@ const queryClient = new QueryClient({ | |||
| defaultOptions: { | |||
| queries: { | |||
| retry: false, | |||
| staleTime: 10 * 60 * 1000, | |||
There was a problem hiding this comment.
I think that we would be OK with:
| staleTime: 10 * 60 * 1000, | |
| staleTime: 1000, |
From what I understand, it would mean that we never query more than once every second, so 3600 requests per hour, below the 5000 limits of GitHub. It's not pretty, but I wouldn't expect anyone to want to refresh more than once per second. But for example in https://master--toolpad.mui.com/_toolpad/app/cl6rqzry10009arlv9sto6qja/pages/ip23ggo I would definitely want to use the refresh button after I label a few issues.
There was a problem hiding this comment.
it would mean that we never query more than once every second
But it's not an unrealistic scenario to have more than one github query on one page. Perhaps we can make it slightly longer, say 10s? I'm also planning to make this configurable at some point (#1093).
There was a problem hiding this comment.
From my perspective, when using a refresh button, we should feel like the results returned are up to date, reliably. I doubt that a staleTime > 1s can allow this UX.
A second concern is that if we set 10s and we fix the over-fetching problem, then we might be blind to a regression.
What I think would be great is to first solve the over-fetching, and then consider caching but to solve API requests that are slow.
There was a problem hiding this comment.
When you call refetch it should ignore cache and always refetch. So the button should always work, regardless of how long we make the staleTime.
There was a problem hiding this comment.
When you call refetch it should ignore cache and always refetch.
Oh yeah, this would be even better.
There was a problem hiding this comment.
@oliviertassinari I took care of the cache busting in the editor. Do you still think we should shorten the staleTime when we have the refetch function?
There was a problem hiding this comment.
@Janpot Per #1163 (comment), I think that we could even remove staleTime if it allows us to not fight with cache invalidation issues, e.g. when editing records. But to be honest, I don't fully understand what this property does so I might not be the best one to answer.
There was a problem hiding this comment.
e.g. when editing records
The current architecture doesn't help with that neither. You'd still have to call refetch after updating the row to make the table up to date. There is no way to do it automatic as the list/delete/update methods are not linked in any way. For that we'd need something like a collection that we can link as a whole to a datagrid (#385)
But to be honest, I don't fully understand what this property does so I might not be the best one to answer.
staleTime sets the time after which data is considered out of date. That doesn't mean react-query will automatically refetch it after this time. It just means that after this time has passed, when certain triggers fire (e.g. window focus) react-query will refetch the query in the background.
Will set it at 0 again. The rest of this PR can stay.
There was a problem hiding this comment.
@Janpot Ok, then agree 10s sounds better than 1s, it could even be 1min 👍
https://tanstack.com/query/v4/docs/guides/important-defaults
There was a problem hiding this comment.
Adding feature request that could help with invalidation: #1184
Adding a
staleTimeof 10m toreact-queryto tame the amount of API calls a bit until we figure out how we should allow users to finetune this behavior.Also adding a way to enable the
react-querydevtools in the application.This also takes care of invalidating cache during editing. The method is straightforward. Alongside the dom we supply a map of hashes of the dom nodes that we have confirmed being saved on the server. We use this hash in the query key to invalidate its cache when serverside node changes. During deployed runtime we don't need to supply these hashes.