Apollo-client: `mutation` does not actually update cache

Created on 14 Mar 2019  路  4Comments  路  Source: apollographql/apollo-client

Intended outcome:

mutations should update the cache, using dataIdforObject for normalization, as indicated in the docs.

Actual outcome:
mutations do not impact the cache in any way

How to reproduce the issue:
The code that leads to updating the cache after a mutation is triggered here:

https://github.com/apollographql/apollo-client/blob/9ce10111afc9c66093a4c262e3272912a45c0784/packages/apollo-client/src/core/QueryManager.ts#L335

However, that code is guarded by a check of

if (fetchPolicy !== 'no-cache')

And mutations are FORCED to have a no-cache fetch policy:

https://github.com/apollographql/apollo-client/blob/9ce10111afc9c66093a4c262e3272912a45c0784/packages/apollo-client/src/core/QueryManager.ts#L153

Is this an oversight in the docs, or in the mutation logic? I'm happy to take a look at resolving, but don't want to contradict intended design.

Thanks!

Versions

System:
OS: macOS 10.14.3
Binaries:
Node: 10.15.1 - ~/.nvm/versions/node/v10.15.1/bin/node
Yarn: 1.13.0 - ~/.nvm/versions/node/v10.15.1/bin/yarn
npm: 6.7.0 - ~/.nvm/versions/node/v10.15.1/bin/npm
Browsers:
Chrome: 73.0.3683.75
Firefox: 65.0.2
Safari: 12.0.3
npmPackages:
react-apollo: ^2.5.2 => 2.5.2

馃檹 help-wanted

Most helpful comment

Thanks for pointing this out @jmreidy. The short story is that the implementation is correct(ish), but both the fetchPolicy for mutations currently only supports the 'no-cache' policy and docs are misleading.

What both the error message and docs are really trying to say is that fetchPolicy's aren't needed when using ApolloClient.mutate, since mutations work in one main way in Apollo Client. The remote mutation is fired first, then the result is updated in the cache. This behaviour is essentially the same as the network-only fetchPolicy, which is what the docs are trying to allude to. So if you don't set a fetchPolicy with your client.mutate call, you're doing the equivalent of a network-only client.query call, in that you're talking to the network first, then updating the cache second.

There are certain circumstances where you might not want a mutation to update the cache, so we wanted to provide a way to disable the cache part of the mutation round trip. Since we were already doing this with a no-cache fetchPolicy in client.query, we re-used the same approach with mutations, but only allow a no-cache fetchPolicy setting. This would have been much more clear if we had dropped the fetchPolicy approach with mutations, and instead just had an updateCache true/false option (true by default to update the cache after a remote mutation, false to disable the update cache part). Fetch policies have been a large source of confusion / frustration for people, and are something that we want to re-consider in a future (major) version of Apollo Client.

If you (or anyone else) is interested in working on a PR to address this in the docs / error message, that would be awesome! The first paragraph of the Bypassing the cache section can just be updated to remove network-only like:

Sometimes it makes sense to not use the cache for a specific operation. This can be done using the no-cache fetchPolicy. The no-cache policy does not write to the cache with the response. This may be useful for sensitive data like passwords that you don鈥檛 want to keep in the cache.

Likewise the above mentioned client.mutate error message can be changed to something like:

Mutations only support a 'no-cache' fetchPolicy. If you don't want to disable the cache, remove your fetchPolicy setting to proceed with the default mutation behavior.

Thanks!

All 4 comments

Thanks for pointing this out @jmreidy. The short story is that the implementation is correct(ish), but both the fetchPolicy for mutations currently only supports the 'no-cache' policy and docs are misleading.

What both the error message and docs are really trying to say is that fetchPolicy's aren't needed when using ApolloClient.mutate, since mutations work in one main way in Apollo Client. The remote mutation is fired first, then the result is updated in the cache. This behaviour is essentially the same as the network-only fetchPolicy, which is what the docs are trying to allude to. So if you don't set a fetchPolicy with your client.mutate call, you're doing the equivalent of a network-only client.query call, in that you're talking to the network first, then updating the cache second.

There are certain circumstances where you might not want a mutation to update the cache, so we wanted to provide a way to disable the cache part of the mutation round trip. Since we were already doing this with a no-cache fetchPolicy in client.query, we re-used the same approach with mutations, but only allow a no-cache fetchPolicy setting. This would have been much more clear if we had dropped the fetchPolicy approach with mutations, and instead just had an updateCache true/false option (true by default to update the cache after a remote mutation, false to disable the update cache part). Fetch policies have been a large source of confusion / frustration for people, and are something that we want to re-consider in a future (major) version of Apollo Client.

If you (or anyone else) is interested in working on a PR to address this in the docs / error message, that would be awesome! The first paragraph of the Bypassing the cache section can just be updated to remove network-only like:

Sometimes it makes sense to not use the cache for a specific operation. This can be done using the no-cache fetchPolicy. The no-cache policy does not write to the cache with the response. This may be useful for sensitive data like passwords that you don鈥檛 want to keep in the cache.

Likewise the above mentioned client.mutate error message can be changed to something like:

Mutations only support a 'no-cache' fetchPolicy. If you don't want to disable the cache, remove your fetchPolicy setting to proceed with the default mutation behavior.

Thanks!

@hwillson Thanks so much for such a prompt and thorough response! I'll leave this issue open so that I can update the docs to add some clarity (and also update the invariant warning). Should be able to get to it in the next week. Cheers!

@jmreidy @hwillson Could you please help me understand the solution more. I am facing the same issue, I am using a mutation with the server side, which contains nested objects that are saved into cache using client.writeQuery.

Please try a recent version of @apollo/client and let us know if this issue is still happening. Thanks!

Was this page helpful?
0 / 5 - 0 ratings