useMutation doesn't roll back optimistic cache.evict

[lorensr]

Successful rollback

When I call this mutation, the following happens:

1. update is run with optimisticResponse, and an item is removed from the list
2. error is received from the server
3. the changes made in update are rolled back: the item is added back to the list

  const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
    update: (cache) => {
      const { reviews } = cache.readQuery({ query: REVIEWS_QUERY })
      cache.writeQuery({
        query: REVIEWS_QUERY,
        data: { reviews: reviews.filter((review) => review.id !== id) },
      })
    },
  })

...

    removeReview({
      variables: { id },
      optimisticResponse: {
        removeReview: true,
      },
    }).catch((e) => {
      if (find(e.graphQLErrors, { message: 'unauthorized' })) {
        alert('👮‍♀️✋ You can only delete your own reviews!')
      }
    })

Failing rollback

When the mutation update uses cache.evict instead of cache.writeQuery, step 3 does not occur.

const [removeReview] = useMutation(REMOVE_REVIEW_MUTATION, {
  update: (cache) => cache.evict({ id: cache.identify(review) })
})

https://github.com/GraphQLGuide/guide/blob/repro-rollback-fail/src/components/Review.js#L132-L141

Versions

System:
OS: macOS 10.15.7
Binaries:
Node: 10.22.0 - ~/.nvm/versions/node/v10.22.0/bin/node
Yarn: 1.22.4 - ~/gh/guide/node_modules/.bin/yarn
npm: 6.14.6 - ~/.nvm/versions/node/v10.22.0/bin/npm
Browsers:
Chrome: 86.0.4240.193
Safari: 14.0
npmPackages:
@apollo/client: 3.1.3 => 3.2.5
apollo: ^2.30.2 => 2.30.2
apollo-cache-persist: 0.1.1 => 0.1.1
apollo-link-rest: 0.8.0-beta.0 => 0.8.0-beta.0
npmGlobalPackages:
apollo: 2.30.2

[benjamn]

@lorensr This is an intentional consequence of #5773, though I understand that behavior can be surprising.

However, when that functionality was implemented, cache.evict did not take Cache.EvictOptions as it does now. I'm open to adding an option to control the shallowness/depth of the eviction, now that it's easier to add named options. Would that help?

[lorensr]

Yeah, if there were an option like cache.evict({ shallow: true }) that rolls back in step 3, that would be great ☺️

Not sure how to name it to make it obvious what to use it for. { rollbackable: true }? 😄 Dunno if there are other scenarios you'd want shallow for.

[benjamn]

@lorensr What if we reused optimistic here?

// Evict from all layers of the cache (the current behavior)
cache.evict({ id, fields: {...} });
// Evict from the top layer only (can be rolled back)
cache.evict({ id, fields: {...}, optimistic: true });
// Evict from the bottom layer only (ignores optimistic layers)
cache.evict({ id, fields: {...}, optimistic: false });

[lorensr]

Looks good! Using an enum for the 3 options with BOTH as default option might be a little clearer, but less neat. I like boolean. Also wonder what the use cases are for optimistic: false

[3nvi]

Any updates on this?

[gmathieu]

Just ran into this today using v3.3.20. An optimistic flag would be really helpful!

[Risbot]

Something news about this optimistic variant?

[benjamn]

@lorensr @brainkim @Risbot @gmathieu @3nvi I did some more thinking, and I believe I've found a way to solve the original problem without passing any additional options: #8829

This is good news, because the same update function typically runs for both the optimistic update and the final non-optimistic update, so it would be tricky to handle the hypothetical optimistic: true option in the non-optimistic case, even if it seemed to solve the problem in the optimistic case.

[pei-wang]

Hi @lorensr,
did u test the this with latest Apollo client? I tested on the code you provided (https://github.com/GraphQLGuide/guide/blob/repro-rollback-fail/src/components/Review.js#L132-L141), it breaks the optimistic deletion.

I am using cache.evict({ id: cache.identify(xxxx) }) to implement the deletion functionality. but the optimistic UI is broken after I upgraded to latest version.
I think it is caused by https://github.com/apollographql/apollo-client/pull/8829/files.

if (this instanceof Layer && this !== limit) {
        evicted = this.parent.evict(options, limit) || evicted;
 }

@benjamn could u help to enlighten me how to make the optimistic update(deletion) work?