apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

Umbrella issue: Cache invalidation & deletion #4

Closed hwillson closed 4 years ago

hwillson commented 6 years ago

Migrated from: apollographql/apollo-client#621

wtrocki commented 5 years ago

@BananaBobby on what version of cache this works? What cache implementation are you using?

In InMemoryCache cache.data field is private. If that works that will be real game changer however direct operations on data are hack due to the fact that watchers will not be notified.

blvdmitry commented 5 years ago

@wtrocki I'm using import { InMemoryCache } from 'apollo-cache-inmemory';. ("apollo-cache-inmemory": "^1.5.1")

This is definitely a hack that I found while playing with cache since reseting store seemed much more like a terrible solution due to the number of requests. Probably there are some side effects like you've mentioned and in my case fetching is rather simple and didn't get affected by that yet.

wtrocki commented 5 years ago

@BananaBobby Made 2 PR's to make this code less hacky:

1) https://github.com/apollographql/apollo-client/pull/4916 2) https://github.com/apollographql/apollo-client/pull/4917

Ad 1.

This will expose private fields so clients can extend cache implementation and experiment with the invalidation strategies proposed above on your local applications before submitting them as PR.

Ad 2.

This one actually provides invalidation for root only elements which is essential for this feature to move on. Once eviction/invalidation will be enabled community can create a separate link that will contain a number of invalidation rules for cache and timers for those - as suggested in many posts above.

Another idea for invalidation will be to also include additional field like __typename but this will not resolve the problem of large cache that is causing issues with apollo-cache-persist.

Abhimanyu310 commented 5 years ago

How far in the future do we see this feature being implemented?

hwillson commented 5 years ago

@Abhimanyu310 We're working on this now (design phase). It's going to take some time to complete, as we're building a new cache (that will support normalization, denormalization, cache eviction / garbage collection, and persistence). We're planning on having this ready by https://summit.graphql.com.

wtrocki commented 5 years ago

@hwillson I know that I might ask too early but do you see apollo-cache-persist eventually being deprecated or it will still have a role in Apollo 3.0?

I'm asking because the community is working on data filtering feature and performance improvements (storing individual values instead of full persist)

hwillson commented 5 years ago

@wtrocki apollo-cache-persist is great, and it's awesome to see the recent traction in that project (thanks to you!). Before you stepped in and started helping out with apollo-cache-persist, we had bounced around the idea of what cache persistence would look like built into Apollo Client. We haven't decided on anything concrete yet (since cache invalidation is a higher priority), but supporting cache persistence out of the box could tie in nicely with the new cache approach we've been considering. @benjamn is leading the design on all of this, so he'll have more to add for sure.

wtrocki commented 5 years ago

@hwillson Thank you so much! Really excited to see progress in that space and willing to help with the implementation of the new spec.

morrys commented 5 years ago

Hi everyone, in this repository I created some libraries that extend the functionality of relay and apollo (caching & offline). In relay-store I natively integrated the persistence through cache-persist and query TTL management. In apollo-cache I used the library cache-persist as ObjectCache of the apollo inmemory cache. I would like your opinion and if possible collaborate in order to improve their integration.

Thanks, Lorenzo

dallonf commented 5 years ago

The new InMemoryCache.evict() method is a very good start, but I'm concerned it doesn't solve the biggest pain point I've had with Apollo Caching, namely paginated lists where fetchMore isn't an option (ex. where the UI allows you to choose a specific page instead of continuously loading more).

For that to work, I think you'd have to be able to evict fields rather than objects - ex. all instances of Query.widgets with any arguments. (maybe WidgetConnection would be marked with the new keyFields: false option to prevent them from being normalized)

studiosciences commented 4 years ago

Agree with @dallonf regarding paginated requests.

I'd like refetchQueries() to refetch active queries and also invalidate any cached queries. Alternately, an invalidateQueries() method could follow the same pattern.

wtrocki commented 4 years ago

For that to work, I think you'd have to be able to evict fields rather than objects

The community have solution: https://www.npmjs.com/package/@jesstelford/apollo-cache-invalidation

I'm using a heavily customized version of that package myself that does build abstractions like pagination that is very specific for my needs.

dallonf commented 4 years ago

@wtrocki Yes, apollo-cache-invalidation has been how I've solved this problem in the past, but it hasn't been updated in over a year and doesn't seem to have kept up with changes to Apollo's internals.

benjamn commented 4 years ago

Field-level eviction is coming in Apollo Client 3, thanks to https://github.com/apollographql/apollo-client/pull/5643.

Funny story: I vaguely remembered reading @dallonf's comment when I implemented that feature, which is why I made sure evicting a field by name would evict all instances of the field within the object, regardless of arguments… but I could not find that comment again for the life of me. Now, at long last, we are reunited!

onderonur commented 4 years ago

I use a similar workaround like some of the suggestions here and just wanted to share it. For example, when I delete a record with a mutation, the mutation resolver returns something like;

{
    __typename: Post,
    id: 1234,
    __deleted: true
}

So, Apollo updates the cached records automatically. Not even need to use the "update" function manually on the client.

The only downside is, the record just doesn't get deleted from the cache actually. I simply use a lightly wrapped useQuery hook and filter out records those have __deleted:true property.

Very, very basic approach, but sometimes it can be messy when dealing with nested data structures.

Instead of filtering these records with a "hacky" way like this and just hiding them from the UI, Apollo might delete the records when some sort of a __deleted field returns from the API. This would result with a "convention" of course. Some of the developers would be irritated, even if the only thing we need to do is returning a recommended result object when a "delete" mutation occurs on the api.

This is a very simple idea, just wanted to share it. Just solves some of my use-cases.

tafelito commented 4 years ago

@dallonf were you able to use evict for the use case you described here

I tried many different ways of implementing that but it never felt right. As you said, it's been the biggest issue I also have with apollo cache

dallonf commented 4 years ago

I haven't gotten to use Apollo Client 3.0 yet, but it seems like the field-level eviction described would be able to handle it.

tafelito commented 4 years ago

Yes I've been trying to wrap my head around and playing with v3 now that is possible.

I wonder tho if you found a workaround for your use case using v2, before migrating everything to v3

dallonf commented 4 years ago

For 1.x, I used https://github.com/lucasconstantino/apollo-cache-invalidation. But if I remember right, it's not compatible with 2.x. So I wound up resetting the entire store whenever anything needed to be invalidated. But this would often throw an unhelpful message: https://github.com/apollographql/apollo-client/issues/3766

Soooo yes I'm very much looking forward to native support in 3.x! 😄

raiskila commented 4 years ago

I likewise ran into that issue when trying to reset the store in 2.x and resorted to throwing away the entire Apollo Client object and recreating it from scratch after some sufficiently complex mutations.

More flexible and reliable cache eviction methods are definitely welcomed.

tusksupport commented 4 years ago

Is this still in roadmap? I need to clear all the permutations of a query, when something is changed by some user and i receive a notification with signalR

danReynolds commented 4 years ago

If anyone on 3.0 wants to try out a beta we have been using for invalidation policies as an extension of the InMemoryCache it should solve some use cases: https://github.com/NerdWalletOSS/apollo-invalidation-policies

3nvi commented 4 years ago

@danReynolds Looks really cool. TTL is something that's def missing from Apollo.

In the repo's example, when an Employee gets evicted, their related EmployeeMessage entities also get evicted. Out of curiosity, how does the repo handle any GraphQL queries that were holding refs pointing to those evicted EmployeeMessage entities? Are they dangling or do they get automatically "pruned" by the package?

My main problem has always been that when you delete an entity, all queries that were having it as a result had issues...

danReynolds commented 4 years ago

@danReynolds Looks really cool. TTL is something that's def missing from Apollo.

In the repo's example, when an Employee gets evicted, their related EmployeeMessage entities also get evicted. Out of curiosity, how does the repo handle any GraphQL queries that were holding refs pointing to those evicted EmployeeMessage entities? Are they dangling or do they get automatically "pruned" by the package?

My main problem has always been that when you delete an entity, all queries that were having it as a result had issues...

The library is just using onWrite and onEvict policies to control when to run cache.evict and cache.modify, so it doesn't clean up those dangling refs automatically. As Ben talked about in this thread: https://github.com/apollographql/apollo-client/issues/6325#issuecomment-656734309, dangling refs should be filtered out of reads now though, so they've been less of a problem for me now. If you did want to make sure they're removed, you could have an onWrite policy like this:

DeleteEmployeeResponse: {
  onWrite: {
    EmployeeMessagesResponse: ({ modify, readField }, { storeFieldName, parent: { variables } }) => {
      modify({
        fields: {
          [storeFieldName]: (employeeMessagesResponse) => {
            return {
              ...employeeMessagesResponse,
                data: employeeMessagesResponse.data.filter(
                  messageRef => readField('employee_id', messageRef) !== variables.employeeId
                ),
              }
            }
          }
        }
      });
    },
  }
},

but in general I've found it okay to leave the dangling refs in cached queries with the new read behaviour.

3nvi commented 4 years ago

@danReynolds That's perfect. Thanks, I somehow missed Ben's PRR on automatic filtering of dangling refs. Good to know that you can confirm no issues were found.

KeithGillette commented 4 years ago

We've adapted @Baloche's deepDeleteAll for our apollo-angular project, which has radically simplified many query updates on deletion since it removes all references to the deleted object. Will this approach continue to work in ApolloClient 3?

tastafur commented 1 year ago

Any news on improving cache invalidation, I've read some interesting version 3 stuff for it but not an easy api to implement