[3.3.0-rc.0] useQuery re-queries other useQuery hooks that share the same query doc

[brendanmoore]

Intended outcome:

Use case where an application uses the same component multiple times, i.e. A on-demand video carousel. That uses a useQuery hook and the same query document but has different query variables every time, i.e. categoryName. The application only queries the carousels that are in viewport with scroll listener/intersection observer, as the user scrolls a previously unseen carousel into viewport it is mounted and should run the query with its unique variables.

Actual outcome:

Every time a new query is expected to run i.e. Carousel enters viewport, it re-runs all previous queries that share the same query document.

How to reproduce the issue:

It is tricky to reproduce with our API as it is not public facing, however downgrading to @apollo/client@3.2.5 fixes the issue.

The resolver for the carousels returns a deterministic unique id (essentially a hash of the input variables) for the result set and there are no "merge" warnings in the console from Apollo like previous issues I have encountered with AC3+. We are not using any reactive variables.

The useQuery options always use:

{
  fetchPolicy: 'cache-and-network',
  errorPolicy: 'all'
}

Could there be some kind of link connected the observables?

Versions

System: OS: macOS 10.15.7 Binaries: Node: 13.7.0 - ~/.nvm/versions/node/v13.7.0/bin/node Yarn: 1.22.5 - ~/.yarn/bin/yarn npm: 6.13.6 - ~/.nvm/versions/node/v13.7.0/bin/npm Browsers: Chrome: 86.0.4240.193 Safari: 14.0

[benjamn]

@brendanmoore It sounds like you don't actually want the cache-and-network fetch policy to be applied every time the data consumed by the other queries is updated in the cache.

If that's accurate, you can switch back to cache-first using the nextFetchPolicy option:

useQuery(QUERY, {
  fetchPolicy: 'cache-and-network',
  nextFetchPolicy: 'cache-first',
})

Even though you're using different arguments for the different queries, the field dependency/invalidation system is "conservative" in the sense that it invalidates all field values for a particular field name (within a given entity object) whenever any field value with the same name (but possibly with different arguments) is modified.

If you're using a cache-first field policy, that over-invalidation is harmless because the other queries will read the same data from the cache again, and no network request will happen. With a fetch policy of cache-and-network, a network request will always happen, even if the cache data are still complete/usable.

This conservative over-invalidation is important because field values with the same field name but different arguments are often logically related to one another, so invalidating only a single field value with particular arguments would not be enough to propagate the full consequences of the update to queries that used slightly different arguments. This wasn't obvious to me at first, but it's something that became clear during the AC3 rewrite. This strategy also means we only need to track one field dependency per entity ID + field name, rather than having a separate dependency for every entity ID + field name + args, which reduces the memory footprint of the dependency system.

[brendanmoore]

@benjamn Thanks for the speedy reply. Just to be clear, from your explanation above this is expected behaviour:

Given I have queried document "A" with arguments "B" and get result "id:X"
When I query document "A" with arguments "C" and get result "id:Y"
Then the previous query "A" with arguments "B" with result "id:X" should invalidate and refetch

I can't shake the feeling that this is not right. Invalidating and re-fetching all the previous queries seems a bit wasteful when each query returns a single Result entity which a deterministic unique id, and seems to explode the number of network requests.

But but but but - I tried out your suggestion of nextFetchPolicy: 'cache-first' it did not resolve the issue however nextFetchPolicy: 'cache-only' did seem to! So thanks for the pointers!

[benjamn]

@brendanmoore Ahh yes, I see what you mean now.

The cache result invalidation system invalidates at the level of individual fields, so queries that did not previously consume the modified fields will not be invalidated. However, in cases where the query document is the same between the two useQuery hooks, there's a much better chance of overlap between (probably all of) their fields, which means writing the second useQuery result will almost always invalidate the first useQuery result, which causes a network fetch with cache-and-network. I agree that outcome seems unfortunate in many cases.

Now you've got me thinking: maybe the over-invalidation strategy only needs to be the default strategy, when you haven't configured keyArgs for the field. Because the cache doesn't "know" anything about the meaning of arbitrary arguments, by default it stores a separate field value for each unique combination of arguments. This keeps the field values safely separated by arguments, but the cache has no knowledge of how those values might be related, so it can't be sure whether one field value should be invalidated when a field value with slightly different arguments gets modified. To play it safe, it invalidates all field values with the same name.

However, if you do configure keyArgs: ["categoryName"] explicitly for the field, perhaps we could respect that choice by invalidating only field values derived from the same key arguments, rather than invalidating all field values with the same field name. You're telling the cache that differences in categoryName should correspond to totally separate field values, so perhaps we should invalidate them separately, too.

I tried a quick experiment where I removed the fieldNameFromStoreName normalization in two places, and I was somewhat surprised that only one test in our whole test suite failed, which is good news for the feasibility of this change. It might not happen until Apollo Client 3.4, but I'm coming around to the idea.

[brendanmoore]

@benjamn Wow thanks for the response and for implementing the new behaviour! This is fantastic 🎉

[hwillson]

Addressed by https://github.com/apollographql/apollo-client/pull/7351 - thanks!