Misleading documentation with regards to query's fetchPolicy allowed policies

[shoooe]

Intended outcome:
Set "cache-and-network" when querying directly from the ApolloClient.

Actual outcome:
Error message:

Error: cache-and-network fetchPolicy can only be used with watchQuery

How to reproduce the issue:
Follow the documentation here and try to query via client.query(...) with fetch policy "cache-and-network".

Version

• apollo-client@

[aldarund]

Yes, its not clear at all why u cant use cache-and-network with query....

[vegetablesalad]

I too have this problem.
Official documentation even gives this example:

export default graphql(gql`query { ... }`, {
  options: { fetchPolicy: 'cache-and-network' },
})(MyComponent);

[macbutch]

I have this issue too. Is there any explanation for the restriction on using cache-and-network with queries? It seems especially strange given that the documentation describes this behaviour as the default (it isn't; the default is no-cache).

[hehoo]

Any update for this issue?

[audiolion]

wondering about this as well, or how to use watchQuery in tandem with query to manually fire a cache-and-network query.

[jlubeck]

Is there a workaround for this?

[theodorDiaconu]

Any update on this ? This is very strange! I expected the query to return initially from cache and make the request in the meantime then the component will get updated with fresh data!

[mikebm]

It appears "cache-and-network" policy does work with the new Query component, but not with client.query as everyone has found. I am not sure if this was the intended behavior or not but it is extremely frustrating given the incorrect documentation and no real example using watchQuery.

[ack210]

@mikebm I was curious about this as well since cache-and-network was working fine with the Query component, but looking under the covers a bit it seems like the Query component actually does use watchQuery: https://github.com/apollographql/react-apollo/blob/1c666276b2206ae882fed3cc0bf6de38ab5432d8/src/Query.tsx#L277

I only looked briefly so I could be wrong, but maybe the Query component implementation could at least serve as an example of how to use the client "properly".

edit: linked to wrong line at first

[cyberdude]

Is this still a thing?

[mikebm]

Is this still a thing?

Yes. See the following code:
https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/ApolloClient.ts#L292-L296

[joelgetaction]

Hi,

Hey @hwillson or @peggyrayzis can we get some movement on this issue please? This is impacting a large number of people and we really need a bug fix. :-)

Thanks!

[mikebm]

@cyberdude this issue is specific to network-and-cache, your example uses network-only which was never an issue. There is code in the apollo client to throw an exception when using client.query with a fetchPolicy of network-and-cache, which is the opposite of what the documentation states.

[cyberdude]

Ahh right, sorry, I was confusing this with another one about missing documentation. Deleting original useless comment I made.

Thanks.

[saernz]

Oh man still not resolved 😭I've had soooo much problem with Apollo docs today regarding local state and thought I was finally on a roll until I tried to implement something that seemed like core functionality only to find it doesn't work :(

Is there a work around for this or do we just need to use network-only for now? 😭

[smolinari]

Duplicated.... #3880

Scott

[PowerKiKi]

The current method signature is:

apollo-client/packages/apollo-client/src/ApolloClient.ts

Lines 331 to 333 in 166f28f

	public query<T = any, TVariables = OperationVariables>(
	options: QueryOptions<TVariables>,
	): Promise<ApolloQueryResult<T>> {

So because query() returns a Promise it can only ever resolve to a single value (on the contrary watchQuery() returns an Observable which can resolve to multiple values over time).

That means cache-and-network cannot be implemented for query() (without introducing a breaking change in the method signature). If you need cache-and-network behavior, then you must use watchQuery().

However we can reduce friction for new users by enforcing this constraint by splitting up FetchPolicy into two distinct types, and updating the docs. Probably something like:

export type FetchPolicy =
  | 'cache-first'
  | 'network-only'
  | 'cache-only'
  | 'no-cache'
  | 'standby';

export type WatchFetchPolicy =
  | FetchPolicy
  | 'cache-and-network';

@hwillson would you accept a PR for this kind of fix ? or would there be a more appropriate approach ?

[hwillson]

@PowerKiKi Yes, we'd definitely accept a PR for this - thanks!

[DavidStummer]

Any workaround/fix for this yet?

[benjamn]

While the error message could be improved, there is no logical bug to fix here.

As @PowerKiKi explained above, the cache-and-network policy makes sense only if you're prepared to receive multiple results. Since the ApolloClient#query method can only produce one result per call by returning a Promise<ApolloQueryResult<T>>, there's no way to get both a result from the cache and a result from the network when calling ApolloClient#query.

In other words, while this error doesn't do a great job of explaining itself (beyond the simple fact that cache-and-network should only be used with watchQuery, which is true), it does a fine job of preventing anyone from mistakenly using the cache-and-network policy in a situation where the client cannot obey that policy.

It's also worth noting that React Apollo uses watchQuery under the hood, which is why the cache-and-network policy makes perfect sense with React Apollo.

Given the misconceptions that have proliferated in this issue, it's very tempting to "solve" this issue by silently replacing cache-and-network with cache-first in ApolloClient#query (perhaps with a friendly warning), since that's almost certainly what folks really want here.

[PowerKiKi]

silently replacing cache-and-network with cache-first in ApolloClient#query

I strongly disagree with that. It will only bring even more confusion to users as they specifically asked for a behavior that can never be valid, but the patch will unexpectedly change the behavior. This patch make Apollo less predictable and more obscure.

The fetch policy is already hard enough to grasp not to introduce a cache-and-network-but-sometimes-just-cache-first.

Also it is very debatable what users actually want to do. In our case we replace cache-and-network with network-only, because we want to be sure to have the latest data from server. Assuming that everybody always want potentially stale data is a very slippery slope. The only person who can properly make the decision is the user by choosing the correct and valid fetch policy for his unique use-case.

Instead I believe we should teach users what is possible or not, and not allow them to have expectations that can never be full-filled.

@hwillson already agreed on that alternative. Was it debated with him before implementing 70fdf25 ? Is there a chance to be reverted ?

[DavidStummer]

@benjamn it makes sense that given ApolloClient#query it returns a promise, it can't perform a cache query then a network query.

I will put my use-case here, maybe it will help clarify what I am trying to achieve. Apologies if it is the wrong place. I need to fetch from cache, then fetch from network if possible (what cache-and-network would do). So, I could query twice using cache-only, and again using network-only, but the crucial thing here that cache-and-network does it that it updates the cache if fresh data comes down, whereas to my understanding querying via ApolloClient#query using network-only doesn't do that. Is there a way to manually update the cache in this situation?

Thanks.

[PowerKiKi]

@DavidStummer if you want cache-and-network, then you have to use watchQuery. Nothing more, nothing less. Anything else would be just re-inventing the wheel.

[DavidStummer]

@PowerKiKi thanks for the clarification. I think I got confused and now realise that network-only does indeed store the data in the cache for later use. Thanks.

[DavidStummer]

I think throwing an error which states something like 'It makes no sense to use cache-and-network with ApolloClient#query, as this function returns promise' would educate new users quickly and avoid this confusion.

[PowerKiKi]

@benjamn your last commit is exactly what I had in mind.

This way TypeScript users are prevented to shoot themselves in the foot, thanks to the strong typing. And JS vanilla users would still get an error on runtime to inform very explicitly that they are doing something wrong.

Thank you for taking the time to implement it.

[timhwang21]

Hi @benjamn @PowerKiKi I'm a bit confused about this change. Now, cache-and-network is apparently no longer permitted for react-apollo <Query/>s. But reading this issue, this seems not to be the intention? As @ack210 mentions a year ago, <Query/> does in fact use watchQuery.

Do react-apollo types need to be updated such that fetchPolicy uses WatchQueryFetchPolicy instead of FetchPolicy? (I can also file a bug report and/or PR if needed.)

edit: never mind! I see it's already fixed here. Thanks for the hard work and excited to be able to use the latest release! 🙂 (Is it worth noting that this change is breaking insofar as types are concerned?)

[benjamn]

@timhwang21 It might be a breaking change if you consider only the type system, but I think it better reflects what would actually happen if you tried to pass fetchPolicy: "cache-and-network" to the query method—it would throw and exception at runtime. So I'm pretty sure there won't be any code that worked before this, but is now broken by this change… though of course there are always edge cases, and we welcome reports of unexpected surprises.

Longer term, I think the watchQuery behavior of subscribing to multiple query results should become the preferred/only way to fetch queries, since it's what React Apollo uses, and it's clearly more flexible than the single Promise<ApolloQueryResult<T>> returned by the query method. We have some good ideas about how to merge those APIs together without totally disregarding backwards compatibility, but that transition will probably happen in the Apollo Client 3.0 timeframe.

[timhwang21]

So I'm pretty sure there won't be any code that worked before this, but is now broken by this change

Just to make sure my own code isn't broken, this is disregarding types right? Just want to make sure <Query fetch policy="cache-and-network"> is valid!

[joelgetaction]

Hi @benjamn - thanks for your hard work on Apollo.

I've had several use cases where the declarative nature of <Query ...> just doesn't work and where we needed to use query() as a result. I guess I don't understand why it's not allowed or supported to use cache-and-network for plain imperative query()? I get that promises only allow a single value to be returned but couldn't this be achieved with some kind of callbacks or observables? I just disagree with the decision to remove cache-and-network from the imperative query function.

I disagree because caching and coding style (declarative vs imperative) are orthogonal to each other and should remain so. For example:

• query() vs <Query> is about imperative vs declarative coding style and both styles have valid use cases
• cache-and-network is about "make my app appear faster by first using a cached value but then refresh with a new network value if it comes back different to what was in my cache"

These two points have nothing to do with each other and should not be connected. One is about coding style, the other is about data staleness. They are not and should not be related.

I really feel uncomfortable that in order to use cache-and-network we're being forced to use the declarative <Query> style. That feels heavy-handed to me and is not a step in the right direction.

I really appreciate you all open-sourcing Apollo and I don't mean to sound like a whiner. I appreciate all your hard work. But I just don't understand why you can't add cache-and-network to the imperative query() function. This PR feels like a step in the wrong direction IMHO ...

Or is the intention that we should use watchQuery in this case? That's still odd because then query() and <Query> differ by more than just imperative vs declarative: the former doesn't use cache-and-network and the latter does, and that feels misleading and like a potential surprise to me. Perhaps <Query> should be renamed to <WatchQuery>?

[benjamn]

@timhwang21 <Query fetch policy="cache-and-network"> has always been valid, because that option gets passed through to the watchQuery method of ApolloClient, which has always accepted the fetchPolicy: cache-and-network option. That said, if you happened to try this in the short span of time between when apollo-client@2.6.0 was released (last night) and react-apollo@2.5.6 was released (this morning), please make sure to update react-apollo if you haven't already!

I get that promises only allow a single value to be returned but couldn't this be achieved with some kind of callbacks or observables?

@joelgetaction That's what watchQuery is for. For what it's worth, if I were rewriting this API from scratch, the watchQuery behavior would be the only option, and that method would be called query instead of watchQuery. If/when that happens (perhaps in Apollo Client 3.0), I think most of what's bothering you will disappear.

Or is the intention that we should use watchQuery in this case?

This is exactly the intention, and I don't think renaming the <Query> component to <WatchQuery> makes sense, not only because it would be a disruptive change for React Apollo users, but also because the component-ness of the <Query> component strongly suggests that it will be updated over time, rather than only ever rendering one result.

For anyone who didn't already know that watchQuery is for multiple results and query is for a single result, you now have both the type system and a runtime invariant to make sure you think about the distinction.

[PowerKiKi]

the watchQuery behavior would be the only option, and that method would be called query instead of watchQuery

This would be a great improvement, albeit breaking a ton of code of course. As newcomers it took us a long time to figure out that the existing query method is mostly useless, and watchQuery is by far the superior choice in all cases.

Until that breaking change happen, it could be possible to emphasized in documentation that query should be avoided in most cases. That would be a sort of "soft-deprecation" ...

[fozzarelo]

watchQuery unhadled promises cannot be caught.

[zhuhang-jasper]

The reason why cache-and-network is not accepted for query is explained here:
#3130 (comment)