skip in useQuery hook doesn't work as expected

[JayMGurav]

Intended outcome:

I was trying to make request conditional, using "skip" in "useQuery" hook, here a brief code of that

here the second query GET_COURSE_BY_ID should be "skipped" if the bought is true,
that's well and good when logged in the console, I get courseDetails as undefined if bought is true,
but the thing is it actually makes the network request and sends back the requested data!!

here's what I get in the console

As expected 👍, courseDetails is undefined if bought is true, But as seen in the network tab it still actually makes a network request and return back the data for courseDetails it so happens that it just makes it undefined later;

The first request - for CHECK_COURSE_BOUGHT

The second request -for GET_COURSE_BY_ID

Actual outcome:

The network request should not be made!!, It should be "skipped" else what is the use of using it. if it is later on just reassigning the data to undefined.

How to reproduce the issue:

Just fallow the code above

Versions

System:
OS: Windows 10 10.0.18363
Binaries:
Node: 12.13.1 - C:\Program Files\nodejs\node.EXE
Yarn: 1.17.3 - F:\softwares\yarn\bin\yarn.CMD
npm: 6.12.1 - C:\Program Files\nodejs\npm.CMD
Browsers:
Edge: 44.18362.449.0
npmPackages:
apollo-link-context: ^1.0.20 => 1.0.20
@apollo/client: "^3.0.0-beta.43",

[joonhocho]

I get the same bug. skip is not respected

[haneetsingh]

I also get the same issue with skip not getting evaluated.

[jengel-rackner]

It's failing for me as well

[macrozone]

skip seem to be broken in apollo-client version 3. using ^3.0.0-beta.50

[dobrynin]

+1

[layerssss]

seems still broken in "@apollo/client": "^3.0.0-rc.4"

[buzinas]

I'm using @apollo/client: ^3.0.0-rc-4.

Use cases:

1. Pass true directly, and it correctly skips.
2. Pass false directly, and it correctly triggers an HTTP request.
3. Pass a variable with a true value, and it correctly skips.
4. Pass a variable with a false value, and it correctly triggers an HTTP request.
5. Pass a variable with false, and then change it to true: it should skip, but it doesn't.

Small runnable reproduction:

https://codesandbox.io/s/skip-does-not-work-rx2rj?file=/src/App.js

I have an array of currencies, and I only want to fetch the currencies which indexes are odd.

Expected

Only queries for odd indexes should trigger an HTTP request.

Actual

After setting the variable to false once, it ignores when I change it to true, so all queries trigger an HTTP request afterwards.

Screenshots

Running console.log(data, currency, skip) correctly shows what I'm expecting:

USD is even, so it skips, and data is undefined.
BRL is odd, so it doesn't skip, and data is returned.
EUR is even, so it skips, and data is undefined.
CAD is odd, so it doesn't skip, and data is returned.
AUD is even, so it skips, and data is undefined.
GBP is odd, so it doesn't skip, and data is returned.

Unfortunately, Apollo triggers an HTTP request anyway, independent of skip:

The only one it respects is the first time I pass true (USD), but EUR and AUD it sends an HTTP request anyway, even if I'm telling it to skip, and even it returning me undefined for data.

[buzinas]

As a work-around, I'm using fetchPolicy: skip ? 'cache-only' : 'cache-first', but this is a nasty bug.

[Taraluktus]

Still having this issue (truthy skip values are ignored) with final version 3.0.0

But I was able to bisect it, maybe that helps the AC contributors (@benjamn @hwillson):

9793b7d9159fae3e82f989be3db6d923fd811893 is the first bad commit
commit 9793b7d9159fae3e82f989be3db6d923fd811893
Author: Ben Newman <ben@benjamn.com>
Date:   Thu Apr 30 12:04:17 2020 -0400

    Refactor ObservableQuery#reobserve to use Reobserver class.

 src/core/ObservableQuery.ts | 198 +++++++++-----------------------------------
 src/core/QueryManager.ts    |  55 ++++++------
 src/core/Reobserver.ts      | 142 +++++++++++++++++++++++++++++++
 3 files changed, 208 insertions(+), 187 deletions(-)
 create mode 100644 src/core/Reobserver.ts

Bisect range was from tag v3.0.0-beta.45 (good) till tag v3.0.0-beta.46 (bad):

git bisect start
# bad: [884b1e5ac35ccad7c8c7778eeaf1f0d7ad40699a] Bump @apollo/client npm version to 3.0.0-beta.46.
git bisect bad 884b1e5ac35ccad7c8c7778eeaf1f0d7ad40699a
# good: [5bd74df074ccd1ec9273e3f4464db0e195e95af2] Bump @apollo/client npm version to 3.0.0-beta.45.
git bisect good 5bd74df074ccd1ec9273e3f4464db0e195e95af2
# bad: [9793b7d9159fae3e82f989be3db6d923fd811893] Refactor ObservableQuery#reobserve to use Reobserver class.
git bisect bad 9793b7d9159fae3e82f989be3db6d923fd811893
# good: [570cb66f9be66407c72a69dbc1ae415d9acb1a3d] Improve Concast comments.
git bisect good 570cb66f9be66407c72a69dbc1ae415d9acb1a3d
# good: [ae3f6d5a5e055aa17e705b8a25d8972f05b19a85] Go back to using a fresh query ID for fetchMore requests.
git bisect good ae3f6d5a5e055aa17e705b8a25d8972f05b19a85
# good: [8aeeba47199ffa49d32107f5a3a0d1f7c0a15e43] Allow in-flight fetchQueryObservable fetches to be cancelled.
git bisect good 8aeeba47199ffa49d32107f5a3a0d1f7c0a15e43
# good: [23b9d07f4f7127a4d2f5d5af2a457bbc841dfd04] Make Concast#{add,remove}Observer public, and allow quiet removal.
git bisect good 23b9d07f4f7127a4d2f5d5af2a457bbc841dfd04
# good: [6acaa370c819eab6e81d524923911bcf7fa4e2b7] Move polling logic from QueryManager to ObservableQuery.
git bisect good 6acaa370c819eab6e81d524923911bcf7fa4e2b7
# first bad commit: [9793b7d9159fae3e82f989be3db6d923fd811893] Refactor ObservableQuery#reobserve to use Reobserver class.

[sapkra]

If it is the exactly the same problem the fix is already merged and will come in the next release:
#6122 (comment)
https://github.com/apollographql/apollo-client/blob/master/CHANGELOG.md

[Taraluktus]

@sapkra Thanks for mentioning it!
I've only tested until 3.0.0-final, will report back if this fixes it for me.

[hwillson]

This should now be fixed in @apollo/client@3.0.1 (reference: #6589). Let us know if not - thanks!

[Taraluktus]

Unfortunately, this still happens for me with 3.0.1 (fetchPolicy "network-only", skip: true)

[vjsingh]

@hwillson I believe this bug is about stopping the actual request, whereas it appears your fix only changes the behavior of onCompleted.

[buzinas]

Yep, still broken @hwillson.

[Taraluktus]

I am working around this for now with manually setting fetchPolicy to "standby" if the condition for skip is truthy, aditionally to setting skip.
But I'm not sure what "standby" is really meant for, did not found any explanation in the documentation.

Nevermind, "cache-only" did the trick ;-)

[buzinas]

I'm using a similar work-around (using cache-only) to make sure fetch isn't triggered, but this should be fixed.

[buzinas]

This is the commit that broke it, btw:
9793b7d

cc @hwillson, @sapkra ^

The onCompleted hook was a separate issue. Please reopen this.

[MichaelBurgess]

@hwillson - still seeing this as an issue in 3.0.1.

[Taraluktus]

@hwillson Yes, please re-open this.

[brunomarchioro]

Hi @hwillson! This problem appears in my projects too, please re-open this issue.

[sirugh]

This is happening for us as well using 3.0.2.

[gentlementlegen]

Still the case with skip: true, the query runs but the data is empty. I think the desired behavior would be not to run the query at all.

[jerelmiller]

Hey all 👋 ! It seems there has been quite a few comments about this issue since it was initially closed. I'm going to go ahead and reopen since judging by its comments, it seems that an issue still persists. I'm hoping we can dig into this a bit further very soon. Thanks for your patience!

[jerelmiller]

From what I can find, this issue appears to be fixed from v3.6.9 on. #9673 appears to have been split out from this issue, which included a reproduction of the bug. This appears to have been fixed in #9823, which was released in v3.6.9. In fact, if you open the reproduction and update the @apollo/client version to any version after v3.6.9, you'll see the network request no longer fires when the query is skipped, as expected.

I'm inclined to say this has been addressed by #9823, however I need some help getting a reproduction and/or failing test to demonstrate the continued buggy behavior. I've created a branch with some additional tests around the skip behavior, but I'm unable to reproduce the problems talked about in this issue with the latest code in main. If there is some condition that isn't covered either by the existing test suite, or the additional tests in my branch, please let me know so we can dig into this further. Thanks!

[rwilliams3088]

+1. Ran into this issue today. As a work around, I added a $skip variable to my query and used the @skip(if: $skip) directive directly in my query.

[STGlancyPS]

I am still finding this to be an issue, have upgraded to 3.7.9

[thomastvedt]

This is really strange... Yesterday this issue re-appeared. But today it works again, and I didn't change anything in my code.
Maybe this is a Heisenbug? 😅

I'm on version: "@apollo/client": "^3.7.9".
My code that failed yesterday, and works today...:

  const { data: dataListItem, loading: loadingListItem } =
    useGetRetailerListItemQuery({
      variables: retailerId
        ? {
            retailerId: retailerId,
          }
        : undefined,
      fetchPolicy: 'cache-first',
      skip: !retailerId, // BUG: open bug in apollo client... will fire even if skip is true
    });

I didn't change anything in my query code, but did some changes around url parameter parsing, maybe affecting how the component is rendered and how many times the component is rendered 🤔.

Note that I'm using graphql-codegen to generate apollo helpers. (not the newest version)

    "@graphql-codegen/add": "^3.2.3",
    "@graphql-codegen/cli": "^2.16.5",
    "@graphql-codegen/introspection": "^2.2.3",
    "@graphql-codegen/typescript": "^2.8.8",
    "@graphql-codegen/typescript-apollo-client-helpers": "^2.2.6",
    "@graphql-codegen/typescript-operations": "^2.5.13",
    "@graphql-codegen/typescript-react-apollo": "^3.3.7",

[AaronWatson2975]

Possibly related to this, I can confirm I'm seeing something similar on 3.7.10, the easiest way for me to reproduce it is to set skip to true and just call refetch, the request still runs and seems to ignore the skip.

I noticed on 3.5.5 I did not see this issue, and on 3.6.0, 3.7.0, and 3.7.10 I do see it (didn't test other versions).

I was about to make a Code Sandbox and open an issue, but then I saw this, and thought I'd post my findings here before opening a potential duplicate.

EDIT: Upon further testing it appears the skip is honoured in the original query, just not the refetch, so it might not be related.

[jerelmiller]

@AaronWatson2975 interestingly, in my mind, I would expect that refetch would execute the query, regardless of skip, since you are explicitly telling Apollo that you want to fetch. I tend to see skip as a means to tell the hook whether to automatically fetch or not.

Unfortunately our docs are a bit ambiguous, so its left up to interpretation. I'll see if we can make find a way to communicate the expected behavior for refetch, fetchMore, etc when skip is true.

Regardless, I think we can all agree that at least the hook should behave properly, so if we are seeing fetches from the hook while skip is truthy, then that is definitely the sign of a bug.

---

@rwilliams3088 @thomastvedt @STGlancyPS if any of you could help create a reproduction with recent versions of Apollo, that would help tremendously. I'm afraid there isn't much we can do without one. I tried creating additional tests for everything I could think of, but I had no luck reproducing the issue.

[AaronWatson2975]

@jerelmiller thanks for the quick response. I had a feeling it might be intended behaviour but I wasn't sure, and I know it worked that way before 3.6. Now that I know it's expected, I can plan for it, so thank you!

Not sure if this helps with the original issue, but I made a 40 ish line CodeSandbox to reproduce my refetch issue, and when I was testing it with the latest version of Apollo Client (at the time of writing this, 3.7.10), I was not able to detect any network requests when skip was true, only detectable when I refetched it, so I was not able to reproduce the original issue here.

Here's the code if anyone else wants to play around with it (it was originally built to show the refetch issue, but if you check the network tab when it loads, you won't find network requests until the timer gets hit and refetch is called). So if you just remove the refetch code you'll notice there aren't any network requests (or that's what I saw when I ran it anyway). Hoping it might be helpful to someone trying to make an example to reproduce the original issue here.

Thanks again! 🙏

import {
  ApolloClient,
  InMemoryCache,
  useQuery,
  ApolloProvider,
  gql
} from "@apollo/client";

const Component = () => {
  const query = gql`
    query GetLocations {
      locations {
        id
        name
      }
    }
  `;

  const { refetch, data: { locations = [] } = {} } = useQuery(query, {
    skip: true
  });

  setTimeout(() => {
    console.log("attempting a refetch, check network tab");
    refetch();
  }, 5000);

  return <p>{`Check network tab.  Location length: ${locations.length}`}</p>;
};

export default function App() {
  const client = new ApolloClient({
    uri: "https://flyby-router-demo.herokuapp.com/",
    cache: new InMemoryCache()
  });

  return (
    <ApolloProvider client={client}>
      <div className="App">
        <Component />
      </div>
    </ApolloProvider>
  );
}

[kylewinkler]

Chiming in here to say still broken 3 years later.

Version: 3.7.9

That's a long time to be asking your user base to implement their own hacky work arounds in order to use your package.

[bignimbus]

Hi @kylewinkler 👋🏻 as @jerelmiller mentioned above, we're keen to fix this but haven't been able to reproduce the issue. Are you using the latest version of @apollo/client? If so, are you able to demonstrate the buggy behavior using https://github.com/apollographql/react-apollo-error-template ? It would go a long way towards being able to address this 🙏🏻

[phryneas]

To add to this: the only reproduction (by buzinas above) we got for this is fixed (I just checked with our latest stable release, 3.7.10), so it's not like this had been around unsolved for three years.
It is very likely that these are different bugs showing similar behaviour - but without any additional reproductions, we cannot do anything.

[kylewinkler]

Sure, I will gather more information in a little bit. I understand that I haven't proven to you that it is the same bug but for the sake of all of our intelligence I will play it safe and incline myself to believe that exhibiting the same behavior as the previous comments and allowing the same workarounds, that this bug is still not fixed lol.

But, time will tell.

[tobiaswaltl]

The problem still exists in Version 3.7.16.

Even worse: I try skip the query in case the variable is not available. Due to the bug the query still fires leading to an error that the mandatory variable is not provided. As a result the onError handler (https://github.com/apollographql/apollo-client/blob/main/src/react/hooks/useQuery.ts#L215) calls setResult which calls forceUpdate (https://github.com/apollographql/apollo-client/blob/main/src/react/hooks/useQuery.ts#L506) which in turn leads to running the hook again ending up in an endless query.

  const { data } = useMyQuery({
    variables: { var: foo! },
    skip: foo == null,
    pollInterval: 5000,
  });

[phryneas]

@tobiaswaltl could you please provide a reproduction of this that shows the problem?

The only reproduction provided in this issue is fixed in recent versions - you might be encountering a similar problem, but we really need a reproduction for it.

[tobiaswaltl]

@phryneas I could narrow it a little bit down:

• It seems that in my case the problem appears when combined with polling. In my case the 1st call should not skip but later ones should. It looks like the polling ignores the skip property.
• It seems that this was introduced in 3.7.11. Below is a small repro. The logs and the network requests show that the requests are fired even though skip is true. When switching to 3.7.10, the requests are not fired.

https://codesandbox.io/s/skip-does-not-work-forked-sdr7d2?file=/src/App.js

[tobiaswaltl]

As a workaround I tried to set the pollInterval to undefined when the condition for skip evaluates to true. But, like skip, this is ignored and the polling is continued. I.e. it seems it's nothing skip-specific causing this bug.

[phryneas]

@tobiaswaltl Sorry for the late response.
Yeah, I think what you see with polling here might be another skip-related bug (The original bug didn't include polling.), possibly the thing with undefined is yet another bug that we'll have to look into.
I'll make a new open issue out of your comment so we can better track that - these kind of issues that combine multiple different similar-ish bugs over years are extremely hard to keep an overview on :)

[jerelmiller]

Hey all 👋

It's been a bit of time since the last reply on this issue. I'd like to give this one more chance at a reproduction before we close this. All signs on our end indicate this has been fixed in recent versions of the client. We've created a separate tracking issue for the bug reported in #6190 (comment) at #11120. I've got a branch setup with some additional tests that all pass with the various ways skip can be set. We've done everything we can do on our end in good faith to try and figure out what's going on.

At this point, if you are still seeing this issue on recent versions of the client, we are going to require a runnable reproduction or failing test case that demonstrates this issue, otherwise we plan to close this as completed. Thanks!

[MrMossevig]

Hi,

I'm not sure if I can give you a runnable reproduction or failing test case, but I came across this issue just now. In my case the skip is not honoured at all, even if it is set explicitly to true:

/* Getting all organizations */
const { result: orgResult, loading: orgLoading} = useQuery(GET_ALL_ORGS, {
  skip: true
});

I am not an expert so I have no idea whether it's a user fault (me) or a problem with the package, but I'm happy to jump on a call on discord or wherever if you want to see.
Using Vue3 with:

"node_modules/@vue/apollo-composable": {
"version": "4.0.1",

"node_modules/@apollo/client": {
"version": "3.9.5",

[phryneas]

Hi @MrMossevig!
I believe your case might be different - the useQuery hook of @vue/apollo-composable is a completely different api than the useQuery hook of @apollo/client.
I believe that useQuery hook just doesn't have a skip option.

[MrMossevig]

Thank you!
I am bit embarrassed that I haven't really realized the difference untill now. But I'm going to hide behind the fact that the identical naming makes it a bit confusing for users :)

Anyways, the apollo-composable useQuery have the "enabled" option that I can use instead.

Again, thank you and have a great day!

[jerelmiller]

Hey all 👋

We are going to go ahead and close this issue since it appears this has been resolved.

If you do encounter a similar bug, please open a new issue with a reproduction and we'd be happy to assist. Thanks!

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.