-
Notifications
You must be signed in to change notification settings - Fork 21
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
[Question] TTLs and lists #39
Comments
Hey @mogelbrod ! First off I love the Star Wars API and thanks for the excellent minimal repro. I took a look at it and I agree that in example 1, I would expect it to evict the |
Thanks for the quick response @danReynolds, much appreciated! I tried out the bugfix/queryKeyArgsTTLs branch in my real code base (couldn't find any way to install or upload the branched version to codesandbox, maybe you know of a way?) but still experienced both issues. Btw, does that mean example 2 works as expected? To me both examples are equally buggy 😅 |
@mogelbrod hmm interesting, I merged it in so feel free to try 2.0.1 to double check. I checked in the codesandbox on 2.0.1 and it was working there. It should fix both example 1 and 2. Let me know if it's still not working and I'll do some brainstorming |
Can confirm that the v2.0.1 release fixed the minimal example, that's great - really appreciating the quick turn around here 💯
The graphql.org SWAPI unfortunately doesn't seem to expose any relay paginated fields that also accept a |
@mogelbrod I did a deep dive on how Apollo treats alias fields this morning and learnt that while they are disregarded at the cache level like I had thought, in our library we were incorrectly trying to look them up by alias name not field name as described here: #41. Feel free to give that branch a go and if it fixes your problem I'll merge it in. Thanks for calling this out! Was super interesting learning more about how aliases are treated and wasn't something we had been using with TTLs before. |
@danReynolds Was just about to reply when your comment popped up 😄 Will check it out! I found two reproducible cases where the list query returning no items after being invalidated:
repro.mov |
#41 seems to have fixed the issue with aliases, nice work! WIth #41 I'm still experiencing errors similar to what the video showcased. I haven't been able to pinpoint the actual cause, could be some timing issue or similar. It only seems to happen when the paginated field has a By the way, would implementing the following be within the scope of this package?
Given that the primary reason why we'd use apollo3-cache-persist is to be able to display stale content while fetching data in the background, not being able to display stale content somewhat defeats the purpose of using it in combination with this otherwise great package. |
Got it I'll keep looking probably later today or tomorrow morning at the continued issue there. As for returning the stale data while it fetches the new data, that sounds possible but would be a larger change that would take a bit longer to get in. In the meantime one hack would be to write an abstraction around your query (such as a react hook if using react) that caches the last result and serves that while fetching new data. It's a great idea though so I hope to make the change soon |
@danReynolds Did you figure out the cause of the issue? I saw the most recent change and was curious if that was an intended fix for the bug 😃 Should I create a separate issue for the "stale while revalidating" request btw? |
Hey thanks for following up! I wasn't able to find the issue and have been on vacation. I'll be back in another 2 weeks and can take a look then. Sorry for the inconvenience! Feel free to make a separate issue for the feature, we may want to discuss the nuances of it because there are a couple different ways to approach it I think. |
Ah in that case I'll stop pestering you, hope you have a great time! Will create another issue soon then 👍 |
Hey 👋
I'd very much like to use this package as it seems to be the first piece of reusable code I've found that could actually make using apollo3-cache-persist viable (preventing the cache from growing indefinitely). Thank you for open sourcing this, and for providing a pretty well written readme along with it 👍
After setting up the package, perusing the readme, and reading this article, there are still some things I haven't really grasped.
I've created a minimal test bed available on codesandbox to try out the package:
Film
s (withtypePolicy: relayStylePagination
), and renders them allFilm
and renders itInvalidationPolicyCache
configured with a defaulttimeToLive
for all types (currently 10 seconds for quick tests)Lists not working: example 1
/
) to a single film (/:id
)/
Expected behaviour: I expected the list to be re-fetched automatically, with either stale data being returned by
useQuery()
(akin tocache-and-network
), or no data (as if the query had never ran before).Actual behaviour: No re-fetching occurs, and an empty list is returned by
useQuery()
. I'm guessing this is due to the cache still containing the dangling references, but should the wholeallFilms
field be removed from the cache just like the individual films where?Lists not working: example 2
/
/
for 10+ seconds for the TTL to expire for the list/:id
/
Expected behaviour: I expected the list to be re-fetched automatically, with either stale data being returned by
useQuery()
(akin tocache-and-network
), or no data (as if the query had never ran before).Actual behaviour: Similarly to the previous example no re-fetching occurs, but this time the list contains the film that visited in step 2.
Possible solutions
Ideally this would work out of the box, but I'm guessing that I need to define some
invalidationPolicies
. Unfortunately I don't really know where to start here, so it would be great if the docs could give an example or two since I'm sure this is a very common use case.Hope this made sense, and thanks in advance!
The text was updated successfully, but these errors were encountered: