Skip to content
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

[Request] Serving invalidated data while refetching (if online) #43

Open
mogelbrod opened this issue Dec 6, 2021 · 9 comments
Open

Comments

@mogelbrod
Copy link

Feature request extracted from #39 (comment).

As I was looking into adopting this plugin I assumed that data which had been invalidated (but not yet garbage collected/evicted from the cache) would still be accessible to apollo client (but marked as stale), behaving similarly as fetchPolicy: 'cache-and-network'. This is currently not the case however - invalidated data seems to be completely inaccessible to apollo.

If the above was possible it would enable using apollo-cache-policies together with apollo3-cache-persist to display stale data while re-fetching in the background (if/when online).

@danReynolds
Copy link
Collaborator

Hey @mogelbrod! Got back from vacation this week and been busy but getting around to this issue. Thanks for your patience! So I thought more about it and I think it's fundamentally rather tricky. The way the Apollo Client works is that the refetching is triggered by the data eviction. When a cache-first query subscription for example has data evicted, it will be unable to satisfy the fields requested in its query and request data from the network. This means that the data must first be removed from the cache in order to trigger the requesting of new data. Working around that setup I think would be pretty hacky unfortunately.

I'm happy to chat more about the technical limitations and brainstorm ideas though!

@mogelbrod
Copy link
Author

Welcome back 👋 Absolutely no worries, hope you had a good time!
Sorry for the late response on my end - don't feel any obligation to replying before 2022 😉

I was worried about that being the case. I have a few questions about how the apollo client/cache interactions work that I would love if you'd be able to clarify. It would help narrowing down possible approaches of implementing this feature.

  1. Is the cache implementation completely responsible for determining if something is resolvable by the cache or not, or can it only manage how the cache is stored?
  2. Does the cache implementation get a request like "resolve this query", or "resolve field X on entity Y with id Z (repeated for each field requested)", or something different?
  3. Does the cache implementation have access to the context of the related query, thus being able to modify properties such as the fetchPolicy?
  4. Are there situations where the client doesn't call the cache implementation as part of a query?

Happy holidays btw!

@danReynolds
Copy link
Collaborator

danReynolds commented Jan 26, 2022

Hey @mogelbrod ! Sorry for the delay, getting back to you on this:

  1. Short answer, yes. The public cache API uses an underlying data store called EntityStore which as you can see here, checks for the presence of fields on its data property.

  2. When a query is made against ApolloClient, it will use a StoreReader abstraction as seen here to resolve the value for that query. This has a layer of memoization in front of it which if stale will resolve fields one by one here: https://github.com/apollographql/apollo-client/blob/45421dc5df901d25b9d1e99b57b5860bdf192ba8/src/cache/inmemory/readFromStore.ts#L349.

  3. The fetchPolicy I think could be modified yes.

  4. If the query's fetch policy is no-cache, then it won't involve the cache altogether.

You've probably moved on from this work since last we spoke so again apologize for the delay. Let me know if you have any thoughts if you're still interested.

@mogelbrod
Copy link
Author

mogelbrod commented Jan 31, 2022

No worries @danReynolds!

Great, so based off that it seems like it should definitely be possible to implement this behavior as an Apollo cache layer, like this package does - right? I'm definitely still interested in this and can spend some time trying to implement a POC to begin with. I'm hoping to start this or next week - will get back to you here once I start running into major challenges 😅

Edit: Looks like #47 will have to be addressed before I can dig into this.

@danReynolds
Copy link
Collaborator

No worries @danReynolds!

Great, so based off that it seems like it should definitely be possible to implement this behavior as an Apollo cache layer, like this package does - right? I'm definitely still interested in this and can spend some time trying to implement a POC to begin with. I'm hoping to start this or next week - will get back to you here once I start running into major challenges 😅

Edit: Looks like #47 will have to be addressed before I can dig into this.

#47 should be addressed now, let me know if there's anything you want to go over. Thanks for taking a look!

@mogelbrod
Copy link
Author

Just writing to let you know that with #47 resolved I'll try implementing this as a POC starting today 👍

@mogelbrod
Copy link
Author

mogelbrod commented Feb 17, 2022

@danReynolds I think I may have found bug/limitation that I'm guessing would be hard to fix without the change in behaviour that this issue proposes.

I've created an expanded version of the original CSB in #47 which adds a dropdown to each individual film page to navigate between them. It's currently not hosted on CSB since I'll be using it when working on #43, but can be cloned from https://github.com/mogelbrod/apollo-cache-policies.

Repro:

  1. git clone [email protected]:mogelbrod/apollo-cache-policies.git
    npm install
    cd example
    npm install
    npm start
  2. Click the reset button to ensure existing cache doesn't affect anything
  3. Open the browser devtools console
  4. Navigate to any film
  5. Dev console should log all 6 movies (showing which records was returned by useQuery()):
    ['A New Hope', 'The Empire', 'Return of ', 'The Phanto', 'Attack of ', 'Revenge of']
    
  6. Select another movie from the <select> dropdown
  7. Dev console should log all 6 movies on each render triggered by useQuery()
  8. Wait 10 seconds
  9. Select another movie from the <select> dropdown
  10. Dev console should keep logging all 6 movies on each render

Actual outcome of (10) is:
The selected movie is omitted from the list while it's being refetched. I'm assuming this happens because the record has been invalidated and thus isn't included in the list (without errors since apollo silently drops unresolvable records by default AFAIK).

My hypothesis is that switching from invalidation to fetchPolicy manipulation would avoid this issue. I'm guessing the harder challenge when doing so would be to still allow for garbage collection of no longer referenced fields.

@danReynolds
Copy link
Collaborator

Yea I agree, invalidation is relatively simple because Apollo will handle all of the fetch logic in response to the eviction. Manipulating the fetch policy to fetch data that we determine is expired but has not yet been evicted is where it becomes more difficult.

@mogelbrod
Copy link
Author

Dove into the API now, and unfortunately it seems like the cache doesn't have direct access to the request/context and thus can't manipulate the fetchPolicy 😞
https://github.com/apollographql/apollo-client/blob/580f9baa0fe020eeaee7a49775e2ee3d3762f1a2/src/cache/core/cache.ts#L17-L26

I guess this library would have to include a custom Apollo link that interacts with the cache for it to be possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants