Urql: Reexecuted and polling operations don't cause `fetching` to be `true` on hooks

[mjadczak]

Describe the bug
When using the pollInterval option with useQuery, the fetching value returned is only true during the first fetch, and not during the automatic re-fetches. I am using the network-only cache policy but I'm not sure if this is relevant.

Steps to reproduce
Use a useQuery hook in a component, specifying the pollInterval option. Add a console.log(fetching) under your hook to see the values of fetching the component receives. Note that true is only logged out when awaiting the initial query.

Expected behavior
During the re-fetching, data has the old data but fetching is true.

Actual behavior
During the re-fetching, fetching is false.

[kitten]

This is not a bug, but expected behaviour. I know, this is probably sounds surprising, but I'll elaborate 😄

We opted to differentiate between "fetching again" and "has fetched initially", because of likely use-cases, where it mostly doesn't matter whether data is being updated in the background.

The exception is obviously executeQuery, which is an explicit call and hence does mean that we need to set fetching: true due to the intent. The same goes for changed queries/variables, which triggers an update and new operation.

The same can't be said for _internal refetches_ where the client or an exchange decides to reexecute the operation. In such a case the hooks don't know about the internal ongoing operation. This is because we deem this a background operation that doesn't need to be surfaced to the user.

I can definitely see this becoming a limitation, but for now it's an intentional one. So I'll mark this as a discussion, just to be sure.

Edit: To talk more about the use cases I've mentioned. Let's say the cache invalidates data and decides to refetch. In that case the best way forward is to show the old data and not let the user decide on whether to show a loading spinner, because it's a background operation/refetch essentially. I've seen some complicated logic with Apollo apps using networkStatus and checking data just to see whether they have to hide the loading screen, while in most if not all cases you probably just don't want to show anything.

The other case is your pollInterval. Our polling is a convenience feature and since it's built-in we also treat those refetches as background fetches. So if you want to instead have the refetches be _explicit effects_ and not _side-effects_ then you can also opt to use useEffect in your component (although we haven't really made that clear)

[mjadczak]

Interesting, I definitely see the point you're making. In my case I wanted to use this flag to drive a small spinner in the corner of my component, just to let the user know that the component is refreshing and about to potentially get some new data.

Is it possible a single fetching flag is not enough to encompass all of the things one might reasonably think of as "fetching"? It seems that the currently intended meaning of fetching is more like "loading", i.e. awaiting some data, as opposed to what I expected it to mean, which is essentially "is there any network activity going on wrt this query". Would you say that for such more "advanced" cases one should integrate lower down, e.g. with a custom hook, or do you think there is scope for exposing some more internal details in the state object returned in the hooks?

[mfix22]

What if a new loading field is introduced, and then fetching can stay as in. No breaking changes necessary 🙂

[kitten]

@mfix22 we've had that before but it caused confusion 😅 I'm overall not quite convinced yet that the actual, internal operation state needs to always be reflected accurately.

In fact, that may cause more confusion. Currently the state inside the hooks reflects that it has triggered an operation but hasn't received a result yet.

Afterwards, any updates are pushed to the hook instead of them being pulled. We could obviously also push the fetching state, but that introduces complexity for almost no gain.

In the worst case scenario the hook can be used to trigger new operations manually, and the client operation stream can be used to get very granular information.

Introducing this by default may actually do more harm than good in terms of unexpected updates of the hook. For instance, with the normalised cache we reexecuted operations that the cache has either updated or invalidated, but the results will likely still be read back synchronously and offline. In concurrent mode and with suspense, pulling this to the foreground may cause complications.

It's not out of the question to introduce a separate hook that gives a More granular status but, who'd actually would want to use two hooks 😅

[kitten]

Closing due to inactivity.

We now have the “stale” flag which is controllable by exchanges. It is set to true currently when cache-and-network triggers another network-only operation after delivering a cached result or when Graphcache delivers a partial result, indicating that another operation will be running in the background.

This explicitly doesn’t apply to other background operations for a couple of reasons:

• We’re trying to keep the fetching state flag an “active” effect and are trying not to let it become a side-effect. This means that we won’t be observing the incoming operations on the client to derive fetching, for simplicity’s sake although this can be written as a hook in user land.
• Invalidating operations in the background may trigger more requests, yes, but as we’re making UI-driven decisions we don’t want those to leak to all parts of an app. Graphcache has a much more solid approach as invalidations due to updates can often be resolved entirely offline due to configs
• “stale” covers all other ground, and doesn’t break our rules as outlined above. It still only concerns the active operation at hand.

Happy to reopen this for new discussions of course 👍 However I do believe that loading states should be avoided as often as possible, hence our general direction in Graphcache, adding only “stale”, and hoping that suspense becomes more useful.