Best practice when a mutation deletes an object about apollo-feature-requests HOT 36 OPEN

I've read the entire discussion in apollographql/apollo-client#899. The recommended approaches are not acceptable for a number of reasons:

• The refetchQueries way: This blatantly violates separation of concern in a UI. If I have two components UserListView and UserGrid that can display users based on GraphQL queries, then the components shouldn't be aware of the other's query.
• The cache.writeQuery() way: The same applies here. It also has a big potential for overwriting data with the wrong contents — in other words, it's asking the client to update data in the same format as the server provides it, and it's not possible to guarantee that this will be consistent across code changes in a large app.
• The "delete flag" way: This is problematic because it doesn't actually evict anything from a cache, and leaves the job of filtering out garbage to UI code which shouldn't need to be aware of it.

The recommended invalidation methods are kludges that don't fit into the layer that they're required for. Invalidation is a low-level concern, but queries and mutations are UI concerns, yet the code has to be commingled.

The cache invalidation problem points to a disconnect in the Apollo universe. Apollo's caching system is aware of object identity, but has no notion of object relationships.

from apollo-feature-requests.

I am having trouble understanding why this problem can be solved by the library's users reengineering their code to respect a delete flag on a record, but can not be solved by the library's authors reengineering their code to respect a delete flag on a record.

Can someone explain what I'm missing here?

from apollo-feature-requests.

@lifeiscontent The Apollo Client 3 beta (@apollo/client) introduces a few ways to handle this:

• cache.evict, cache.release and cache.gc: https://www.apollographql.com/docs/react/v3.0-beta/caching/cache-interaction/#garbage-collection-and-cache-eviction
• cache.modify: apollographql/apollo-client#5909

This will all be better documented shortly.

from apollo-feature-requests.

I must say, coming from redux, that's the most painful part so far.
And for the solution
"You should adapt your data model to have a deleted flag"
Doesn't that go against the principle of graphql to not adapt to the need to each and every clients needs?

I guess this will be fixed with 3.0
#33

from apollo-feature-requests.

The reason I say it's the "real problem" is that if Apollo had cache eviction then this would be automated inside Apollo Cache.

I'm thinking something along the lines of refetchQueries managed internally by Apollo instead of UI code. I agree that the solutions above are insufficient but without cache eviction there's not much you can do short of building your own local cache on top of Apollo cache 🤯

So if you remove a user from UserGrid and then evict it, Apollo having tracked internally all queries where the user appears would then mark them for refetch on the next access only (I think this is very important for performance). This is assuming that your UserListView is in another screen that gets re-mounted later and so re-executes the query. If it's on the same screen (mounted next to UserGrid) you'd most likely have to refetch yourself anyways but I think that's a simple acceptable compromise.

These are hard problems (having worked on an immutable caching solution myself) and require a significant amount of tracking metadata attached to the entities inside the cache, not to mention batching updates etc but it's doable. I feel it's a must if Apollo Cache wants to reach full maturity for large scale (enterprise grade) applications. Apollo cache already has some pretty gnarly performance issues (due to normalization) that we had to overcome by putting our hierarchical entities de-normalized in Redux after fetching and using from there during reconciliation.

from apollo-feature-requests.

To everyone subscribed: this has probably gone unnoticed but one of the best features of Apollo 3 just got removed: apollographql/apollo-client#6289

from apollo-feature-requests.

I have a GET_ENTRIES query that accepts a few variables (such as first, after, etc) and queries all the Entries. For every entry in this list I have an option to delete this item. The delete mutation is name DELETE_ENTRY that accepts the Entry id and removes it from the database.

Right now, the only way I can use the update function in the mutation to delete the Entry from the list is to pass the GET_ENTRIES and all the variables that were used in the GET_ENTRIES query. If one variable is different then the entries list does not get updated. This doesn't look like a good pattern as the mutation could be deeply nested and no decent way to get the same variables from the top query.

I also have a GET_ENTRY query but since it's not being used anywhere it's not in the ROOT_QUERY and thus I can't query for the exact entry and remove it.

Any tips?

from apollo-feature-requests.

Just to be clear, the following works very well and doesn't require to refetch the query:

update(cache, {data: mutationData}) {
  if (mutationData) {
    const removedMatchId = mutationData.removeMatch;
    cache.modify('ROOT_QUERY', {
      matches: (matches: Reference[], helpers) => {
        const removedMatchRef = helpers.toReference({
          __typename: 'Match',
          id: removedMatchId,
        });
        return matches.filter(({__ref}) => __ref !== removedMatchRef.__ref);
      },
    });
  }
},

This is a big improvement compared to read/writeQuery because you won't have to specify parameters, but still very error prone because developers can forget to update certain queries.
That's why I think that it would still be a good practice to also evict the entry from the cache and eventually refetch the remaining affected queries. It should also be a good practice to log it when this happens.

The problem with this approach is that it won't refetch those queries until you re-use them, so if any of those is already in an active view the user will miss the update.
Even worse, it will break subsequent cache updates until the query gets finally refetched.

from apollo-feature-requests.

I found an approach that is concise and suits my needs is creating a special type ItemDeleted which will be returned when the item has been deleted and then evict cache from typePolicies:

schema.graphql

extend type Mutation {
  items: [Item!]!
  deleteItem(id: ID!): ItemMutationResponse!
}

type Item {
  id: ID!
  # etc...
}

type ItemDeleted {
  id: ID!
}

type ItemMutationResponse {
  code: Int!
  success: Boolean!
  message: String!
  item: ItemDeleted
}

resolvers.ts

    deleteItem: async (_, { id }) => {
      // ...
      return {
        success: true,
        message: 'Item deleted successfully',
        property: { __typename: 'ItemDeleted', id },
        code: 200,
      }
    },

Using this "special type" allows me to react to incoming:

client.graphql

mutation DeleteItem($deleteItemId: ID!) {
  deleteItem(id: $deleteItemId) {
      code
      success
      message

      item {
        __typename
        id
      }
  }
}

client.ts

    // inside typePolicies
    ItemDeleted: {
      merge(_, incoming, { cache, readField }) {
        cache.evict({
          id: cache.identify({
            __typename: 'Item',
            id: readField<string>('id', incoming),
          }),
        })
        cache.gc();
        return incoming;
      },
    },

This is basically the same as doing the usual, but avoiding having to repeat update all over:

     await deleteItemMutation({
        variables: { deleteItemId: id },
        update: (cache, { data }) => {
          cache.evict({
            id: cache.identify({
              __typename: 'Item',
              id: data?.deleteItem.item?.id,
            }),
          });
          cache.gc();
        },
      });

Overall, this approach is efficient, straightforward, and helps maintain a consistent state in the cache after a item is deleted. It provides a clear separation of concerns between the mutation resolver and the cache update logic.

from apollo-feature-requests.

I'm playing with cache.evict and cache.gc and that's really awesome because affected queries get invalidated and the next time you access them they get automatically refetched from the network.
But I would like to be able to configure this behaviour (ideally on a global basis and on a per-query basis), for two reasons:

1. I want the UI to reflect the new state immediately, at least for some of those queries
2. If I need to read one of those queries (for example because of a subscription who wants to update it or because I want to add local state), cache.readQuery will throw an error:

Invariant Violation: Dangling reference to missing User:3 object

from apollo-feature-requests.

To everyone subscribed: this has probably gone unnoticed but one of the best features of Apollo 3 just got removed: apollographql/apollo-client#6289

Does apollographql/apollo-client#6350 help you out?

from apollo-feature-requests.

that readField signature is kind of backwards, in most APIs I know of you put the object first, and key second

from apollo-feature-requests.

@mchong-teal As I understand it, this only works for deletions of objects are that are referenced as child objects by other objects, not for top-level objects.

For example, let's say groups have users; i.e. group.users. This means that if removeUserFromGroup returns a new group object, the cache for its key will be updated to the new group object, and the removed user will no longer be included in that group's group.users. So that works.

But let's say your request is a mutation called deleteGroup. That response can't return anything, because the group doesn't exist anymore, and there's no way to tell Apollo that it's gone. (It could return the deleted group with a deleted: true flag, which is the workaround proposed in the old issue thread. See my original comment for why this isn't a good solution.)

There may be a workaround. I believe you could define a "dummy" singleton object that has an ID and always has all groups as its children. Normally a query to find groups would be something like:

query {
  groups { id, name }
}

Instead of this, you'd do something like:

query {
  allGroups {
    id
    groups { id, name, ... }
  }
}

Now a deleteGroup mutation can defined as deleteGroup(id: String!): AllGroups. The response will then update the cache with the full group list.

The downside is that every time you delete a group, you have to return the entire group list to the client. For larger datasets, this may not be practical. The dummy singleton object also feels pretty unnatural.

from apollo-feature-requests.

@hwillson I spent time building out a real world (A Medium.com clone) example using Apollo.

https://github.com/lifeiscontent/realworld

There were are still a couple of issues in the repo, specifically when favoriting an article and unfavoriting an article.

in the case of unfavoriting (from the favorites list) it was pretty straight forward, but in cases where favorites are being added on a page that doesn't manage the list, I don't see a clear way of updating the cache, do you guys have a solution to solving this kind of problem?

from apollo-feature-requests.

The same happens with cache.modify:

const removedMatchId = mutationData.removeMatch.id;
const cacheId = cache.identify({
  __typename: 'Match',
  id: removedMatchId,
});
if (!cacheId) {
  throw new Error('Cannot generate cacheId');
}
cache.modify(cacheId, (_, {DELETE}) => DELETE);

from apollo-feature-requests.

AFAIK the query gets invalidated and refetched.

from apollo-feature-requests.

@bkoltai I see, you could try a nested cache.modify()? Like the first code snippet in this:

#266 (comment)

No id needed for a root query field like viewer. Could return INVALIDATE or DELETE for productsConnection. https://www.apollographql.com/docs/react/api/cache/InMemoryCache/#modifier-functions

from apollo-feature-requests.

Based on some of the comments on the end of the original issue, it seems like a good approach is to flag deleted records and then omit them in your UI. Another clever approach from @maierson involved marking queries for refetch rather than refetching them immediately. Are either of these approaches to the problem something that would be considered for inclusion in Apollo itself?

from apollo-feature-requests.

The real problem is that Apollo Cache has no cache eviction. Is that still the case?

The lack of it is effectively a memory leak that needs to be handled periodically by invalidating the entire cache and refetching.

from apollo-feature-requests.

That is also true. I don't think that's the "real problem", but it is a problem.

from apollo-feature-requests.

I am just encountering this issue and am wondering if the following solution will work for me. However I'm relatively new to graphql and apollo, so wondering if others can provide some feedback on thinking this through/has tried something similar and can offer some guidance:

• ATM, my mutation responses include a field that allow me to 'query' the data that has been updated in the response, eg. an 'addUserToGroupResponse' might include a 'user' field (which then has a 'groupMemberships' field), so a mutation can include a 'query' for the updated list of groups in the response.
• For deletions, the response would similarly include fields necessary to make those queries (e.g. 'removeUserFromGroupResponse'). The 'deleteUser' response might include a 'users' field, to get an updated list of users.

I can see that maybe in the case I delete a user that I've queried by id (or some other specific parameters) this might break? Also the 'removeUserFromGroupResponse' might need a 'group' field, e.g. if I am deleting (or adding) from a 'group' component. Has anyone tried a similar approach that can offer some guidance? How might this scale?

from apollo-feature-requests.

@atombender, great, thanks for that. I can see now how this would be an issue for larger datasets, and the singleton object does seem a bit strange. I think for now I'm not going to bump up against scaling issues, but will keep following the developments on this thread. Thanks again.

from apollo-feature-requests.

Hey Guys just joining this discussion after recently diving into Apollo and GraphQL. Is there an update for this feature request elsewhere or a better solution to handling delete mutations w/ cache updates other than the three options @atombender mentioned above?

from apollo-feature-requests.

@hwillson is there a solution for this? I would expect you would just write null to the cache for the id you want to evict.

e.g.

cache.writeData({id: 'Post:1', data: null });

from apollo-feature-requests.

@darkbasic

but still very error prone because developers can forget to update certain queries

I had the same issues with redux and the way I combated the "developers forgetting" part was to evict the item itself. Then every reference to the deleted item from any query, would just return null (since the related item got deleted) and I would to filter out the null items through a selector.

This created memory issues though, since my cache/store always had references to items that weren't existent in the cache. I don't know if it's possible to follow a similar path, but in order to combat what you said, the only way is for Apollo to create a way to:

1. Delete an Item from the cache (currently exists with cache.evict)
2. Delete all references to the deleted item (doesn't exist, since cache.gc() only clears top-level cache fields)

eventually refetch the remaining affected queries

I don't understand why a refetch would be needed. You just need to clean them up of stale references. Am I missing something here?

from apollo-feature-requests.

The "delete-all-references" approach would probably work in 99% of the cases, but I would still make it optional for the following reason: let's say that you made a forum application and a user asks to be deleted, but you don't want to delete all of his posts. You clearly cannot delete all posts with the author field pointing to the deleted user. Then what should it point to? What if our backend decides that it should point to a default user? Or maybe it simply returns null? The client cannot know and thus has to refetch. On the contrary, if you're sure that deleting an item forces the deletion of all the related entities, then you can safely remove every associated reference.

from apollo-feature-requests.

Hmm.. I think that this scenario is a bit advanced and would warrant either some custom logic or even a page refresh. It's a bit complicated to handle through code without losing track of what to update.

I understand what you mean though and I do agree that what I proposed should be optional. Essentially, what I'm proposing is a cache.gc({ deleteStaleRefs: true }) with the default value being false. It should never be true since the underlying tree traversal might be expensive CPU-wise if you have a big cache

from apollo-feature-requests.

I also think that we need an isOptimistic parameter inside the update function, because if we want to evict something from the cache we cannot risk doing so with the optimistic response: if it turns out not to be what we were expecting then we will be forced to refetch the query once again.

from apollo-feature-requests.

It's not documented, but you already have access to that in the cache.modify as the 3rd argument:

export declare class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
    
  // ...
  modify(dataId: string, modifiers: Modifier<any> | Modifiers, optimistic?: boolean): boolean;
  // ...
}

guess you can use that instead of evict since evict is using (or will soon be using) modify under the hood

from apollo-feature-requests.

@3nvi the cache.modify optimistic parameter is to specify if the modification is supposed to be optmistic or not, but how am I supposed to know if update doesn't expose such information?

from apollo-feature-requests.

@hwillson if we .evict an object from cache that's in a result list of an active query, what happens? Does the item get removed from the list query result or does it become null, or does the query get refetched? (The docs don't clarify what happens yet)

from apollo-feature-requests.

cache.evict(): https://www.apollographql.com/docs/react/caching/garbage-collection/#cacheevict

and if the item is in a list, cache.modify(): https://www.apollographql.com/docs/react/caching/cache-interaction/#example-removing-an-item-from-a-list

from apollo-feature-requests.

I can't seem to get a singleton object to get evicted when deleting or adding a new entry.

Use case is as follows:
Given a structure like

{
  viewer {
    productsConnection(filters: ..., paginationOptions: ...) {
      totalCount
      edges { ... }
    }
  }
}

When I {add|remove} a product, I want to invalidate viewer.productsConnection to force a refetch. This is mainly due to the fact that the connection is typically called with all sorts of arguments for filtering and pagination.

I've tried cache.evict({id: "ROOT_QUERY.viewer.productConnection"}) but that doesn't seem to work.

I've also tried cache.modify({ id: "Viewer", fields: { productConnection() {...} } }) but my function for productConnection never executes.

Maybe this is because of the arguments that are always present for the field? Do I need to do something special to tell the cache that I want to access the field no matter what arguments it has been executed with?

from apollo-feature-requests.

I'm trying to use evict({ id }) but it's not clear to me what the id should be. Can anyone point me to the documentation on that?

from apollo-feature-requests.

@bkoltai apollographql/apollo-client#6795 (comment)

@azamanaza check out:

https://www.apollographql.com/docs/react/caching/cache-configuration/#generating-unique-identifiers
https://www.apollographql.com/docs/react/caching/cache-interaction/#obtaining-an-objects-custom-id

from apollo-feature-requests.

Thanks @lorensr. I'm not sure how the comment you linked to relates to my question though. My issue is that I need to evict ROOT_QUERY.viewer.productsConnection* where viewer is a singleton (doesn't have a unique ID) and productsConnection might have 0 or more arguments. I would like to evict all entries for productsConnection under the viewer singleton object in the cache.

From what I understand, the comment you linked only explains how to evict fields of the ROOT_QUERY singleton, but how do I access fields of the viewer singleton

from apollo-feature-requests.