Calling refetch fires off a network request when skip is true

[knedev42]

Calling refetch ignores the skip: true option for the query hook. The full config of the query hook is:

fetchPolicy: "cache-and-network",
nextFetchPolicy: "cache-first",
notifyOnNetworkStatusChange: true,
variables: { ... },
skip: true

Intended outcome:

Calling refetch when skip is true should not produce a network request.

Actual outcome:

Calling refetch when skip is true produces a network request. While the request is in flight the loading state is false, when the data is fetched it's not rendered (not passed to the data prop). The reason why skip is true in this case is lack of certain variables, that would filter a large data set. There's a button on the page that can "refresh" the data, which calls the refetch function. Previously (on v3.1.3) refetch was throwing an error, instead of firing off a network request (which was also a bad behavior and was something that we handled in a weird way). It would be nice if it can just respect the skip property and not do anything if it's true.

Versions

npmPackages:
@apollo/client: ^3.3.19 => 3.3.19

[martdavidson]

Yes please, seeing the same behaviour.

@brainkim Do you want a reproduction made for this, or is it straight forward enough to rig up on your own?

[brainkim]

One thing you can possibly do is check whatever state you’re using for the skip property and checking that before calling refetch(). I could fix this, but I‘m hesitant to do so because people might be relying on refetch() working even when skip is true. Can you feel me?

[knedev42]

This is exactly what I'm doing at the moment. That said, the current behavior is clearly broken even if we ignore the fact that it fires off a network request when skip is true (the data is never received by the hook even though a request was in flight and has completed). Plus the behavior was different in the previous minor version (it threw an error instead of firing off a request)...

What's the point of versioning if things aren't getting fixed because someone might rely on a non-sensical/buggy behavior?

[martdavidson]

@brainkim Thanks for the response! I can definitely do what you're suggesting, but I agree with @knedev42

That said I can understand your hesitance but I think the skip API is so clear that you're going to have more issues not respecting what are (I think) reasonable expectations that the network request does not get made if skip is true.

[brainkim]

@knedev42 Oh I missed that part, was the behavior changed between 3.1 and 3.3? I guess I’m okay with changing the behavior then.

[martdavidson]

@brainkim Coming back to this as the workaround isn't really possible for my usecase. The skip logic is complex and in a custom hook that does some other data fetching, and the refetch is called through a react Context which has no idea about all the skip logic or how to recreate it (nor should it).

Anyway, all this to say: would love to see this fixed or at least know that it's on some kind of roadmap.

[Minishlink]

@brainkim @hwillson Is there not a difference between skip and the standby fetchPolicy? I tend to think that if skip is true, requests are all skipped, but if it is in standby, the user can still manually update the cache or refetch
Either this or the documentation needs updating :)

Uses the same logic as cache-first, except this query does not automatically update when underlying field values change. You can still manually update this query with refetch and updateQueries.

[brainkim]

Is there not a difference between skip and the standby fetchPolicy? I tend to think that if skip is true, requests are all skipped, but if it is in standby, the user can still manually update the cache or refetch

I don’t think it’s wise to distinguish standby fetch policy from the skip boolean. It simplifies a lot of things.

This behavior will soon be reverted. The correct way to deal with refetch() being called when skip is true is to simply read the skip variable.