You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Investigation: fine-grained async control over caching behaviour #4693
One approach would be to have another async function that runs before the queryFn (and maybe one that runs after it ?) that would skip calling the queryFn if something other than undefined was returned. Here, people could read from an async storage (and potentially write to it in the "after" hook). The query could also stay in fetchStatus: 'idle' because we haven't triggered the queryFn yet.
Combined with the possibility to customize the in-memory cache (see #4687), people would be free to keep the size of the in-memory cache small.
a load function that can be sync or async, and returns a record of a CachedState shape, in the case or react-query, it would be just the cached state
an onCacheLoads callback that may be used to imperatively set the state from cached data.
a getDeadline function that receives the Succeeded state, and returns the state time.
The run of my producer (query for react-query) first calculates the hash of the run (the query key in your case), then checks if there is a non expired cache entry, then set's it as current state (if not already).
I hope these insights provide any help.
Im not linking my lib to avoid being targeted as an ad.
One thing we could do is add an onQuery callback to the QueryCache itself. This would be similar to the onMutate callback of useMutation - it would run before the queryFn.
It could be async, and when something other than undefined is returned, we would use that as the result for this query instead of calling the queryFn.
The query could also stay in fetchStatus: 'idle' because we haven't triggered the queryFn yet.
Not sure this is possible or desired.
and maybe one that runs after it ?
This is what the onSuccess callback could be used for.
In combination with #4687 , we could so something like this:
const queryClient = new QueryClient({
queryCache: new QueryCache({
cache: createMyCustomCache(),
onQuery: async (query) => {
const data = await readFromAsyncStorage(query.queryKey)
return data ?? undefined
}
onSuccess: (data) => {
writeToAsyncStorage(query.queryKey, data)
}
})
})
Reacted by Seph Soliman and mohammad mohiuddin mostafa kamal akib
@TkDodo I think this topic connect to the one we have discussed previously.
Potentially we could introduce a persister argument to the query options.
If provided, persister could be run instead of a queryFn. persister could have following implementation on a high-level (and be provided by @tanstack/local-storage-persister etc.):
functionpersister(context,queryFn,persisterOptions){// 1. Check if data in `QueryCache` for given `queryKey` exists (means it's been restored already or fetched). If yes, then we skip the persistance layer logic and move to point 4// 2. Query the persistance layer for stored data by `context.queryKey`// 3. If stored data matched the `predicate` or `staleTime` from `persisterOptions` just return it and skip the `queryFn`// 4. If data don't match or does not exist, run `queryFn`// 5. store data returned by `queryFn` in the persistance layer (possible configuration in `persisterOptions`)// 6. return fetched data}
Benefits of such implementation:
framework agnostic persistance plugins (just a js function), needs to be written once and serves all adapters
fine-grained control over which queries to persist
per-query persistance options
possibility to persist everything with defaultOptions
possibility to persist branch of queryKey with setQueryDefaults
removes initial QueryCache restoration delay and connected logic, cause query state would be restored lazily just-in-time when query needs the data
it does not matter if persistance layer can be accessed synchronously or asynchronously as it's run in a promise (no need for a separate sync-stoage-persister)
Drawbacks:
Query data will not be immediately available, Promise needs to be queued even for localStorage access
This means that app will be in a loading state for a brief second. But i think that this should be fine as users should be handling loading state anyways.
We could even start those new per-query persisters as a wrappers for queryFn as a POC without extending API layer.
Promise needs to be queued even for localStorage access
Yes, we have to make this async, which is fine 👍
This means that app will be in a loading state for a brief second. But i think that this should be fine as users should be handling loading state anyways.
so when the persister runs and restores on page load, we don't subscribe the observer (even though it's already created). That makes sure your app renders, but it will be in pending state and fetchStatus: idle. Then, after restoring is done, we'll trigger the subscription, which might trigger a refetch - or not, if the data we now have is already fresh.
So I think this drawback would be fine - Whatever we do should not run if we already have fresh data in the cache. That's why I like the idea of a wrapper around the queryFn that could be exposed by the persister package, because it could not only take care of the read operation, but also the write back to the cache.
My other idea was similar to yours, except that I was leaning more towards an onQuery method (similar to onMutate on mutations). The idea was to run this function right before the queryFn, and if it returns a resolved Promise, we'd put that in the cache and don't run the queryFn. If the returned promise fails, we'll go on and run the queryFn. But this wouldn't handle the "writing back" part - I was thinking this would then need to happen in onSuccess of the QueryCache or so.
So I kinda like the wrapper idea. It would also keep the size of the core package the same because it would come from the persisters. However, I think it would be harder to set it globally unless you use a default queryFn...
Im not really versed in developing a library as a newbie but im very invested in seeing a solution as I have currently a react native project that could greatly benefit from separate query caches. The queries can contain several megabytes of base64 strings and loading/storing them all at once is slow with my current implementation. My hacky solution has been so far to create separate queryclients and bringing the provider down the component tree but that solution is hard to maintain.
I like all proposals so far.
If i understood the proposal correctly my ideal solution would be to add persiter field to useQuery hook options object that would take in an object similar to persistclient configuration. The hook would first read persister value with get callback and set the result as initialData if successful. If not successful continue with regular fetch function. If stale time is configured and hasnt elapsed it would set the initialData as data and consider the query as success. If the stale time has lapsed or persister fails to provide info the queryFn would fire based on other rules in options object. Callbacks would then fire based on queryFn results. If the queryFn is a success then persister set function would be called and new result stored in persister. If invalidateQuery is called with matching key then persister del function would be called. This would fail the persister get step next time the query is used and query would force fetch anyway.
Am i correct that the proposal is at least in same spirit? Cause if yes then that would be an amazing addon to the library.
Option b I also like is if the persistclient get/set functions would optionally take in a serialized key so that each query could be seperatly handled and the developer has the responsibility by writing the correct get/set/del functions. Not exactly sure what the API would look like. Perhaps createPersister(separateKeys?: boolean) or something? Not sure.
In any case i wish you guys all the inspiration. Looking forward if the functionality can be implemented in any way so that the lambs are well and the wolves have eaten.
Reacted by Dominik Dorfmeister 🔮 and Kilian Finger
Activity
incepter commentedon Dec 23, 2022
Here are the cache insights from my lib:
The
CacheConfigaccepts a:loadfunction that can be sync or async, and returns a record of a CachedState shape, in the case or react-query, it would be just the cached stateonCacheLoadscallback that may be used to imperatively set the state from cached data.getDeadlinefunction that receives the Succeeded state, and returns the state time.The
runof myproducer(query for react-query) first calculates the hash of the run (the query key in your case), then checks if there is a non expired cache entry, then set's it as current state (if not already).I hope these insights provide any help.
Im not linking my lib to avoid being targeted as an ad.
TkDodo commentedon Dec 26, 2022
One thing we could do is add an
onQuerycallback to theQueryCacheitself. This would be similar to theonMutatecallback ofuseMutation- it would run before thequeryFn.It could be async, and when something other than
undefinedis returned, we would use that as the result for this query instead of calling thequeryFn.Not sure this is possible or desired.
This is what the
onSuccesscallback could be used for.In combination with #4687 , we could so something like this:
DamianOsipiuk commentedon Feb 3, 2023
@TkDodo I think this topic connect to the one we have discussed previously.
Potentially we could introduce a
persisterargument to the queryoptions.If provided,
persistercould be run instead of aqueryFn.persistercould have following implementation on a high-level (and be provided by@tanstack/local-storage-persisteretc.):Benefits of such implementation:
defaultOptionsqueryKeywithsetQueryDefaultssync-stoage-persister)Drawbacks:
immediatelyavailable, Promise needs to be queued even forlocalStorageaccessloadingstate for a brief second. But i think that this should be fine as users should be handlingloadingstate anyways.We could even start those new per-query persisters as a wrappers for
queryFnas a POC without extending API layer.TkDodo commentedon Feb 4, 2023
Yes, we have to make this async, which is fine 👍
so when the persister runs and restores on page load, we don't subscribe the observer (even though it's already created). That makes sure your app renders, but it will be in
pendingstate andfetchStatus: idle. Then, after restoring is done, we'll trigger the subscription, which might trigger a refetch - or not, if the data we now have is already fresh.So I think this drawback would be fine - Whatever we do should not run if we already have fresh data in the cache. That's why I like the idea of a wrapper around the
queryFnthat could be exposed by the persister package, because it could not only take care of the read operation, but also the write back to the cache.My other idea was similar to yours, except that I was leaning more towards an
onQuerymethod (similar toonMutateon mutations). The idea was to run this function right before thequeryFn, and if it returns a resolved Promise, we'd put that in the cache and don't run the queryFn. If the returned promise fails, we'll go on and run the queryFn. But this wouldn't handle the "writing back" part - I was thinking this would then need to happen inonSuccessof the QueryCache or so.So I kinda like the wrapper idea. It would also keep the size of the core package the same because it would come from the persisters. However, I think it would be harder to set it globally unless you use a default queryFn...
TkDodo commentedon Feb 14, 2023
@DamianOsipiuk would you like to give the wrapper function a try? I'd imagine something like:
that way, you could create your custom wrapper by moving the
withPersistdeclaration somewhere central:elraito commentedon Feb 14, 2023
Im not really versed in developing a library as a newbie but im very invested in seeing a solution as I have currently a react native project that could greatly benefit from separate query caches. The queries can contain several megabytes of base64 strings and loading/storing them all at once is slow with my current implementation. My hacky solution has been so far to create separate queryclients and bringing the provider down the component tree but that solution is hard to maintain.
I like all proposals so far.
If i understood the proposal correctly my ideal solution would be to add persiter field to useQuery hook options object that would take in an object similar to persistclient configuration. The hook would first read persister value with get callback and set the result as initialData if successful. If not successful continue with regular fetch function. If stale time is configured and hasnt elapsed it would set the initialData as data and consider the query as success. If the stale time has lapsed or persister fails to provide info the queryFn would fire based on other rules in options object. Callbacks would then fire based on queryFn results. If the queryFn is a success then persister set function would be called and new result stored in persister. If invalidateQuery is called with matching key then persister del function would be called. This would fail the persister get step next time the query is used and query would force fetch anyway.
Am i correct that the proposal is at least in same spirit? Cause if yes then that would be an amazing addon to the library.
Option b I also like is if the persistclient get/set functions would optionally take in a serialized key so that each query could be seperatly handled and the developer has the responsibility by writing the correct get/set/del functions. Not exactly sure what the API would look like. Perhaps createPersister(separateKeys?: boolean) or something? Not sure.
In any case i wish you guys all the inspiration. Looking forward if the functionality can be implemented in any way so that the lambs are well and the wolves have eaten.
DamianOsipiuk commentedon Feb 14, 2023
@elraito Yeah, this is the rought idea.
@TkDodo Yeah I can prepare a POC. But probably next week.