Apollo-client: cache.write warns "Cache data may be lost" when removing properly normalized object from list in cache

[yngwi]

Intended outcome:

When resolving a @client mutation that removes an item from a list in the cache, the item should be removed without a warning.

Actual outcome:

The client reports the following error:

Cache data may be lost when replacing the notifications field of a Query object.

To address this problem (which is not a bug in Apollo Client), define a custom merge function for the Query.notifications field, so InMemoryCache can safely merge these objects:

  existing: [{"__ref":"Notification:6gNxJHP6IEqlycPgJL8pl"},{"__ref":"Notification:RgcN77oZvCyoVetQeEDV5"},{"__ref":"Notification:zY96QaR5A0AYYeSxbHwNx"}]
  incoming: [{"__ref":"Notification:6gNxJHP6IEqlycPgJL8pl"},{"__ref":"Notification:zY96QaR5A0AYYeSxbHwNx"}]

How to reproduce the issue:

I've created a sandbox illustrating the issue. Clicking on "add" adds a new notification that will remove itself after a short while by calling the removeNotification mutation (see the notification resolver and the Notification component).

As far as I can tell the object can be properly normalized and I followed the basic documentation on how to use cache.writeQuery.

Versions

  System:
    OS: Linux 4.19 Debian GNU/Linux 9 (stretch) 9 (stretch)
  Binaries:
    Node: 14.1.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /workspace/node_modules/.bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
  npmPackages:
    @apollo/client: 3.0.0-rc.5 => 3.0.0-rc.5 
    @apollo/link-error: ^2.0.0-beta.3 => 2.0.0-beta.3 
    apollo: ^2.28.3 => 2.28.3

[yngwi]

The test sandbox also shows the following error Warning: Can't perform a React state update on an unmounted component. but as far as I can tell this should be caused by the problem discussed in #6209

[benjamn]

@yngwi Thanks for the reproduction!

In this case, there's one more step you need, just before calling cache.writeQuery:

  cache.evict({
    // Often cache.evict will take an options.id property, but that's not necessary
    // when evicting from the ROOT_QUERY object, as we're doing here.
    fieldName: "notifications",
    // No need to trigger a broadcast here, since writeQuery will take care of that.
    broadcast: false,
  });

This eviction tells the cache that the existing Query.notifications data can be safely ignored, because you're handling its transformation yourself. If you do not evict the field, the cache has no way of knowing the new data you are writing is a filtered version of the existing data, so it warns about potential loss of the existing data.

Another way to remove elements from a list without having to do readQuery, evict, and writeQuery is to use the low-level cache.modify API:

cache.modify({
  notifications(list, { readField }) {
    return list.filter(n => readField("id", n) !== id);
  },
});

[yngwi]

@benjamn Thank you for your reply, what you write makes perfect sense. So is there a drawback to cache.modify as it sounds much simpler than the read/write approach I used and also the additional cache.evict step?

Regards,
Daniel

[benjamn]

Well, cache.modify was originally (#5909) designed to help with deletion of elements from lists, so it's pretty good for that! It does have a few caveats, though:

• As the name suggests, cache.modify can only transform existing field data, and cannot create new fields like writeQuery or writeFragment can.
• The format of the data received by the modifier function is the internal InMemoryCache format (like what you see if you call cache.extract()), not the usual GraphQL result objects, so you have to deal with Reference objects, options.readField, and other implementation details.
• Removal is simple, but if you want to _add_ elements to a list, you need to make sure you're adding the same type of data that the list already contains. Usually, this means you need to use cache.writeFragment to write your data into the cache first, then insert the Reference it returns into the list using cache.modify (see https://github.com/apollographql/apollo-client/pull/6289#issuecomment-634160039 for an example of that).
• Custom read or merge functions are not called during cache.modify, so you may need to check your cache policies before modifying a field.

[yngwi]

Thank you for your clarifications, this helps me very much.

[rickco75]

@yngwi Thanks for the reproduction!

In this case, there's one more step you need, just before calling cache.writeQuery:

  cache.evict({
    // Often cache.evict will take an options.id property, but that's not necessary
    // when evicting from the ROOT_QUERY object, as we're doing here.
    fieldName: "notifications",
    // No need to trigger a broadcast here, since writeQuery will take care of that.
    broadcast: false,
  });

This eviction tells the cache that the existing Query.notifications data can be safely ignored, because you're handling its transformation yourself. If you do not evict the field, the cache has no way of knowing the new data you are writing is a filtered version of the existing data, so it warns about potential loss of the existing data.

Another way to remove elements from a list without having to do readQuery, evict, and writeQuery is to use the low-level cache.modify API:

cache.modify({
  notifications(list, { readField }) {
    return list.filter(n => readField("id", n) !== id);
  },
});

Thank you, this really helped me out! Isn't there a way to have the cache automatically update when items are added and deleted? Seems like this would be something very common using GQL