[Question] TTLs and lists

[mogelbrod]

Hey :wave: 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:

• Interacts with the public graphql.org Star Wars API (implements relay connections)
• One route which queries a relay paginated list of Films (with typePolicy: relayStylePagination), and renders them all
• One route which queries a single Film and renders it
• InvalidationPolicyCache configured with a default timeToLive for all types (currently 10 seconds for quick tests)
• No apollo3-cache-persist for now since I'd like to get the basics working first

Lists not working: example 1

1. Navigate from the list (/) to a single film (/:id)
2. Wait 10 seconds for the TTL to expire for the list
3. Navigate back to /

Expected behaviour: I expected the list to be re-fetched automatically, with either stale data being returned by useQuery() (akin to cache-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 whole allFilms field be removed from the cache just like the individual films where?

Lists not working: example 2

1. Load /
2. Stay on / for 10+ seconds for the TTL to expire for the list
3. Click on any film to navigate to /:id
4. Navigate back to /

Expected behaviour: I expected the list to be re-fetched automatically, with either stale data being returned by useQuery() (akin to cache-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!

[danReynolds]

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 allFilms field on returning to the root page /. After some debugging, I found an issue in the library for your use case with custom keyArgs specified on the Query and addressed it here: https://github.com/NerdWalletOSS/apollo-cache-policies/pull/40. Let me know if you have thoughts/concerns there.

[mogelbrod]

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 😅

[danReynolds]

@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

[mogelbrod]

Can confirm that the v2.0.1 release fixed the minimal example, that's great - really appreciating the quick turn around here 💯 I'll dig further into why my real project isn't working tomorrow (during EU office hours 😅), but the only differences I can tell right now between the working codesandbox and broken codebase (referred to as real below) are:

1. The real field represents a search query, so it has the additional variables query: String! and type: SomeEnum
2. The real version passes a non-empty keyField array to the relayStylePagination helper for the above reason:

   search: relayStylePagination(['type', 'query']),
   

3. The real query uses GQL aliases to query the same field with different types (could this possibly be the reason why it doesn't work?):

   query Search($query: String!, $first: Int) {
     playlists: search(query: $query, type: playlist, first: $first) {
       pageInfo { endCursor hasNextPage }
       edges {
         cursor
         node { id title }
       }
     }
     tracks: search(query: $query, type: track, first: $first) {
       pageInfo { endCursor hasNextPage }
       edges {
         cursor
         node { id title }
       }
     }
   }
   

The graphql.org SWAPI unfortunately doesn't seem to expose any relay paginated fields that also accept a query param or similar, so I can't provide a publicly available reproduction for this. If this info isn't enough to go by I'd happily try to provide a minimal repro using our private API through another channel.

[danReynolds]

@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: https://github.com/NerdWalletOSS/apollo-cache-policies/pull/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.

[mogelbrod]

@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:

1. Aliasing the paginated field (codesandbox)
2. Relay paginated content that has one or more keyFields eventually seem to result in the same issue (codesandbox). This was a bit harder to repro, but check the attached video for an example. Apollo cache can be inspected via window.cache.data.data

https://user-images.githubusercontent.com/150084/142452404-c8164b41-6151-4ec5-8f3d-d40e40de4a39.mov

[mogelbrod]

#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 keyField'ed arg like $query - if I remove the variable from the GQL query it seems to stop happening.

By the way, would implementing the following be within the scope of this package?

I expected the list to be re-fetched automatically, with ~either~ stale data being returned by useQuery() (akin to cache-and-network)...

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.

[danReynolds]

#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 keyField'ed arg like $query - if I remove the variable from the GQL query it seems to stop happening.

By the way, would implementing the following be within the scope of this package?

I expected the list to be re-fetched automatically, with ~either~ stale data being returned by useQuery() (akin to cache-and-network)...

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

[mogelbrod]

@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?

[danReynolds]

@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.

[mogelbrod]

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 👍