-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RTQK: Better manual cache updates or clearer info on limitations (eg first query errored, resetApiState) #1529
Comments
Thanks for your feedback! I understand your concerns, but from a documentation clarity perspective, I have no idea of how the following scenario could be any more clearly defined as it's spelled out in the docs as well as in the type hints + jsdoc blocks.
In those scenarios, it should be very obvious that there is no cache entry because you: 1. never had cached data due to a failed request, and 2. you reset the api state (in this case, you should be aware that you called I do believe the docs mention the pitfalls and encourage preferring tag invalidation over manual cache entries. IMO, these are just tools that can be used in certain scenarios. If we were to support a 'force create entry' type of feature, what would you imagine that API looked like? |
Hi, thanks for the response. I suppose I might've gotten a bit too used to react-query's approach.
Well from the end dev point of view, ideally it would look similar to react-query. So for context, here's a query in react-query:
The equivalent to
The difference being that this sort of thing goes in an onSuccess callback, instead of a tags array. And just setting the query manually, whether or not it exists just looks like this:
(naturally, these hooks can then be set up as custom hooks to avoid repeating this code on each component that uses the query) Here's their docs: https://react-query.tanstack.com/reference/QueryClient#queryclientsetquerydata "setQueryData is a synchronous function that can be used to immediately update a query's cached data. If the query does not exist, it will be created." "After successful changing query's cached data via setQueryData, it will also trigger onSuccess callback from that query." This approach, solves any possible issues related to failed requests or reset caches. The query is considered successful and (if I recall correctly), the So they try to be a bit agnostic about whether the user prefers to Of couse, different tools have different philosophies and react-query may just want to have a broader scope by not focusing necessarily on a request based approach (although, I would've expected rtk-query to also follow that route given that it's linked to redux). As for how the api would work internally, unfortunately I'm not yet well versed enough in rtk-query to be able to give a good answer. I can't say for sure if a lot of people would want to have this in the lib to warrant consideration, but I get the impression that many people would want it. EDIT: Just noticed RTKQ uses the query name for the |
Let's start from the beginning: I had initially a second argument that would take another callback to calculate a potential initial value, but I removed it again. Because honestly, this seems very smelly to me. RTK Query is an API cache, in the sense that it always should just mimic the data that the server has access to. It is not meant to store any data beyond that. And the main way of getting that data into the cache should always be by just getting it from the server. If you start adding data in there manually, it kinda feels that you are not using it as a cache, but as some kind of weird key-value-store. And when you want any kind of global store, it is right at your fingertips: Redux. There will be a way to "put" data into the store, using the rehydration options, but this is also explicitly not meant to get any hand-created data into the store, but just to restore data that already was in there in the past. |
I get your point. It's just that it's common for people to use the result of a POST request, without triggering a new GET request for that resource. After all, what the POST returns also comes from the server, it doesn't have to be "hand created data". I personally enjoy having the option of using it as a kind of key-value-store. In certain, even if uncommon, situations I can set a query using the result of a POST request, and only have it fetched if that page is being loaded. I wouldn't want to manage a request version with the highly convenient custom hooks, while at the same time managing state using the rest of the redux api just because I might not want to use a request. I would want the hooks to work for any case (which is the case with react-query). Anyway, I guess this comes down to personal preference and library philosophy. I only understood clearly that cache entries are only created with a successful request (and only then would Anyway, thanks for your help :) |
There's also another somewhat related edge case that might be relevant. If an api returns Again, it's an edge case. One case that comes to mind is firebase's currentUser request that returns And also, you wouldn't be able to manually update the cache. |
From the back of my head, that should be fixed in 1.7 and only show that behaviour on |
An additional use case: with server-side push/WebSockets, we could potentially receive new entities that haven't been explicitly added into the cache as the result of a query. Attempting to add them is a no-op currently |
@MaybeSacred There's a PR in the works for this: #2266 . We'd appreciate any feedback you have on the API design and behavior! |
Hi,
I noticed that the docs are being changed to better convey how to perform manual cache updated here 1525, but I feel that the current implementation can be a bit misleading and lead devs to unwittingly create bugs.
I think that most people think of manual cache updates as changing a cache entry without triggering a new query request (ie invalidating tags), and for the most part this is possible.
However, if the first query request errored, or the entry hasn't been created, or the dev calls
resetApiState
, then truly manual cache updates will all fail.Here's a use case. Let's say that the dev is using rtkq to manage the current user session (maybe user sessions isn't the most common use case, but I'm sure other cases could have similar issues):
1 - Presumably, on app load there'll be a
getCurrentSession
request.2 - The user logs in and the request returns the session data. The dev doesn't want to invalidate the getCurrentSession tag and only wants to replace the cache entry with the data from the login request.
3 - Now the problem is that if the first
getCurrentUser
request for some reason failed (the cache has no data, only the error property) or if on logout the dev callsresetApiState
, manual cache updates won't work and login will be broken.This in some sense also breaks optimistic updates, technically the data will show up, but it only after tag invalidation and its respective request, which could be a confusing behavior.
I'm aware that the docs do indicate 'if a cache entry exists', but it might not be obvious that this applies to a query that errored on first (only) request and after
resetApiState
calls. It's very easy to assume that manual cache updates would always work no matter what.resetApiState
call) and that you should always useinvalidatingTags
whatever strategy you use to update cache entries.Anyway, thanks for you work.
The text was updated successfully, but these errors were encountered: