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 Remove experimental cache.modify method. #6289 (comment) 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

[spathon]

Thanks @benjamn your responses were the best explanations I found and helped a lot.

Thought I would add an updated version of the current cache.modify

cache.modify({
  fields: {
    notifications(list, { readField }) {
      return list.filter((n) => readField('id', n) !==id)
    },
  },
})

Could be good to use cache.evict({ id: cache.identify(item) }) as well to clear the item from cache as well.

[benjamn]

Although cache.modify is still the best way to work around this problem, I wanted to share that we're considering a new overwrite?: boolean option for cache.writeQuery and cache.writeFragment, which can be used to silence these warnings and just replace any existing field values (which is almost always what you want when you're using the readQuery-modify-writeQuery pattern), without having to silently evict anything: #7491 (comment)

[spathon]

That would be helpful.
Is there any downside of just using cache.evict without modify on a small simple list?
It gave me the same result (gone from the list) but with refs still in cache without it's items in the cache.

[srowe0091]

This warning also appears when using the updateQuery from useQuery. Unfortunately I can not use cache.modify because my operation could both remove and add new items to the collection. I am setting the data for the updateQuery so I am basically overwriting the whole array with "new" items, but really the delta could just be removing 1, adding 2 (just an example). It does seem like the workaround would be to cache.modify first, and the writeQuery, doing 2 operations but that just seems excessive when updateQuery does what i need it to do.