2.6.0 @client @export directive fix has caching issues

[jwld]

Relates to #4530

Intended outcome:

Using the @export directive in a query should also fetch from the cache when possible.

Actual outcome:

Currently it seems to do a full refetch every time.

How to reproduce the issue:

The issue can be seen here. Moving between the ids sets loading to true every time.

Versions

Can see in the sandbox above ^

[benjamn]

Thanks for helping test the betas/RCs, noticing this issue, and providing a clear reproduction @jwld!

My conclusion after playing with the example is that this is working as designed, though the exact behavior is certainly open to discussion. This commit introduced the behavior of forcing a refetch whenever variables change and the query has some non-@client parts and the fetchPolicy is not cache-only (in this case it's cache-first, the default policy). Here's the latest version of the code, with the same logic.

One thing we've improved in 2.6.0 is that the networkStatus received by the React component is no longer always NetworkStatus.loading or NetworkStatus.ready (see #4765), so in this case you could at least distinguish between the initial load and subsequent loads, even though loading === true in both cases. Specifically, the networkStatus ends up as NetworkStatus.setVariables === 2 after you click one of the buttons, even though it was initiated by a refetch, because the variables are changing, which overrides the NetworkStatus.refetch status here.

Looping in @hwillson in case he has any additional thoughts.

[jwld]

Thanks for the info! This behaviour does seem pretty odd to me.

If you pass the variable as an actual variable (by nesting Query components, rather than exposing it via @export) then the cache behaves as I'd expect it to - only showing loading state when it's unable to fetch from the cache. If this @export directive is intended to negate having to nest queries like this then I'd expect the behaviour to be identical.

Also, on a slight tangent (maybe something for another issue) - is there a way to skip a query based on a variable exposed via @export? Seems like quite an important use case, and is again something that can be done when nesting Query components (by using the skip prop).