Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

"Skip" property of useQuery hook has no effect #3492

Open
limekiln opened this issue Sep 13, 2019 · 42 comments
Open

"Skip" property of useQuery hook has no effect #3492

limekiln opened this issue Sep 13, 2019 · 42 comments

Comments

@limekiln
Copy link

Intended outcome:

Using the skip property of useQuery, I expect the query not to be executed as long as skip === true.

Actual outcome:

No matter which value skip has, the query is always executed.

Version

System:

  • OS: Linux 5.0 Ubuntu 19.04 (Disco Dingo)
  • Binaries:
    • Node: 12.3.0 - /usr/local/bin/node
    • npm: 6.11.2 - /usr/local/bin/npm
  • Browsers:
    • Firefox: 69.0
  • npmPackages:
    • apollo-cache-inmemory: ^1.6.3 => 1.6.3
    • apollo-client: ^2.6.4 => 2.6.4
    • apollo-link-http: ^1.5.16 => 1.5.16
    • react-apollo: ^3.1.0 => 3.1.0
@wooller
Copy link

wooller commented Sep 13, 2019

Same issue here, when setting skip to true, the query is still executed

@hwillson
Copy link
Member

Would one of you be able to provide a small runnable reproduction for this? Thanks!

@wooller
Copy link

wooller commented Sep 17, 2019

@hwillson I just tried a really simple version in codesandbox and it works fine. It could be related to the fact that im using an AWS-appsync client instead of an apollo-client. So im hesitant to post my exact code.

here is the sandbox of it working anyway https://codesandbox.io/embed/funny-field-1oeyk

maybe @limekiln can provide his set up if its more inline with a regular apollo setup

@jthistle
Copy link

jthistle commented Dec 9, 2019

+1, I am still having this issue. onCompleted doesn't fire as expected, but looking at my 'network' tab in the inspector, I'm still seeing a request to my GraphQL endpoint. @apollo/client 3.0.0-beta4.

@honglish
Copy link

honglish commented Dec 9, 2019

This is still an issue for me too, and I am using "@apollo/react-hooks": "^3.1.3"

All you need to do is use skip: true, and you will notice that it still executes.
I've currently bypassed this by using pollInterval instead
ex: pollInterval: true ? 0 : 5000

@DennisBaekgaard
Copy link

I can confirm I have the same issue.

"@apollo/react-hooks": "^3.1.2"
"react-apollo": "^2.5.5"

@limekiln
Copy link
Author

Sorry for the late reply and not supplying a code example. For my current version setup, everything related to the skip property seems to work fine now. I will post the information again for anyone who is still facing this issue:

System:

  • OS: Linux 5.0 Ubuntu 19.04 (Disco Dingo)
  • Binaries:
    • Node: 13.5.0 - /usr/local/bin/node
    • npm: 6.13.4 - /usr/local/bin/npm
  • Browsers:
    • Firefox: 71.0
  • npmPackages:
    • @apollo/react-hooks: 3.1.3 => 3.1.3
    • apollo-cache-inmemory: ^1.6.5 => 1.6.5
    • apollo-client: ^2.6.8 => 2.6.8
    • apollo-link: ^1.2.13 => 1.2.13
    • apollo-link-error: ^1.1.12 => 1.1.12
    • apollo-link-http: ^1.5.16 => 1.5.16
    • react-apollo: 3.1.3 => 3.1.3

@tagamma
Copy link

tagamma commented Feb 17, 2020

Still having this issue with:

 "@apollo/react-common": "^3.1.3",
 "@apollo/react-components": "^3.1.3",
 "@apollo/react-hooks": "^3.1.3",
 "apollo-cache-inmemory": "^1.6.5",
 "apollo-client": "^2.6.8",
 "apollo-link": "^1.2.13",
 "apollo-link-error": "^1.1.12",
 "apollo-link-http": "^1.5.16",
 "react-apollo": "^3.1.3",

useQuery(QUERY, { variables, skip: true }) still runs the query.

@imeugenia
Copy link

Hello,
If the issue is still relevant, you might try to import useQuery from "react-apollo" rather than "@apollo/react-hooks". I can confirm that skipping works when imported from this package.

@FacundoGFlores
Copy link

FacundoGFlores commented Mar 18, 2020

@imeugenia which version of react-apollo? just tried 3.1.3 and didn't work

@vjsingh
Copy link

vjsingh commented Mar 23, 2020

@imeugenia @FacundoGFlores I'm also having this issue. Perhaps it's a bug in 3.1.3 since that's the same version I'm using.

Using "react-apollo" doesn't fix the issue for me either.

@trdaya
Copy link

trdaya commented Apr 3, 2020

The issue exists with fetchPolicy as cache-and-network.
However, it works as expected with fetchPolicy as network-only

@vjsingh
Copy link

vjsingh commented Apr 4, 2020

@trdaya I'm actually trying to use it for a query with the default fetchPolicy, and it's not working. Version 3.1.3 as I mentioned earlier

@nathan-omaorg
Copy link

I'm on 3.1.5 and also experiencing the same problem. Have tried changing fetchPolicy from cache-and-network to network-only as well as making sure the import is from react-apollo.

@DanSchoppe
Copy link

I'm bumping into this after client.resetStore(). The query in question skips successfully up until I do a store reset, then makes an invalid query because it's querying when it's supposed to be skipping. Are the rest of you doing any resetStore?

Curious if the query refetch logic that is run on store reset doesn't contain the necessary logic to obey the skip option?

@DanSchoppe
Copy link

In QueryManager::reFetchObservableQueries I updated this conditional to squelch refetch for queries that have a truthy options.skip:

        if (
          fetchPolicy !== 'cache-only' &&
          (includeStandby || fetchPolicy !== 'standby') &&
          !get(observableQuery, ['options', 'skip']) // <----- Here
        ) {
          observableQueryPromises.push(observableQuery.refetch());
        }

This did fix the error I was bumping into. I couldn't figure out where this change should go (ObservableQuery::refetch?). But hopefully this is a clue as to what's going wrong.

@derLalla
Copy link

I'm facing the same error. I am also getting a network error 400: "variable $jwt of non-null type String! must not be null."
The whole idea of skipping a query is therefore invalid and breaking my code ...

@davydkov
Copy link

davydkov commented May 29, 2020

Yes, I'm hitting the same on @apollo/client: 3.0.0-beta.50
apollographql/apollo-client#6190

The whole idea of skipping a query is therefore invalid and breaking my code ...

@derLalla Agree 😒

@278kunal
Copy link

I am facing the same issue on apollo-client: ^2.6.8

@ziimakc
Copy link

ziimakc commented Jun 3, 2020

same problem with @apollo/client: 3.0.0-beta.50

@mdecurtins
Copy link

Same problem with @apollo/react-hooks: "~3.1.5" and apollo-client: "~2.6.4". I initially tried to use useLazyQuery for my case but I could never get it to show the loading state; it was like the query had already been executed regardless. I get the loading state just fine with useQuery and I thought I'd try the skip option, but just saw this behavior that the query is executed anyway even if skip: true.

@ziimakc
Copy link

ziimakc commented Jun 14, 2020

If i set skip to true - it works, but if i use dynamic variable skip: !state.startDate it's not

@buzinas
Copy link

buzinas commented Jun 15, 2020

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:

image

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:

image

image

image

image

image

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.

@Taraluktus
Copy link

No news on this? We still have this issue in the latest RC9, are forced to use a pre-beta.50 version.

@chillyistkult
Copy link

chillyistkult commented Jun 26, 2020

It actually broke in 3.0.0-beta.46. Last working release is 3.0.0-beta.45!

@vjsingh
Copy link

vjsingh commented Jun 29, 2020

No updates on this? Is there a workaround? The following workaround suggested in #6190 doesn't work for us since we need to use refetch:

fetchPolicy: skip ? 'cache-only' : 'cache-first'

@Taraluktus
Copy link

Taraluktus commented Jun 29, 2020

Two other issues regarding skip are tracked in the apollo-client repo for the 3.0 milestone, e.g.:
apollographql/apollo-client#6342

@vjsingh
Copy link

vjsingh commented Jun 29, 2020

@Taraluktus Thanks for the response. If that's the case, this issue should be closed, and that other issue should encompass the client-side problem as well, and be referenced here as the canonical issue.

@zkwentz
Copy link

zkwentz commented Jul 7, 2020

For those like me...

According to @apollo/client tests, skip only works with polling, see here:

https://github.com/apollographql/apollo-client/blob/master/src/react/hooks/__tests__/useQuery.test.tsx#L514

So for me, where I was doing:

useQuery(QUERY, { variables, skip: true })

...the query was still made, and that is intended, skip, skips on poll.

@buzinas
Copy link

buzinas commented Jul 7, 2020

the query was still made, and that is intended, skip, skips on poll.

There are two wrong assumptions here:

  • If so, the docs should explain that, I shouldn't need to look at the tests code to assume how it's supposed to work.
  • Also, that's not really true, if you pass skip: true, it doesn't make the query. It only makes the query if you pass a dynamic value to skip, and between renders you change from false to true. So, it's definitely a bug.

This is a big issue and people logged it in many different repos, and the Apollo team should at least give us a response. I tried to debug their code to come up with a PR, but it's too deep, hard to grasp, so I can't help if there aren't any guidelines. For now, I'm just using a hack to work-around the bug.

@zkwentz
Copy link

zkwentz commented Jul 7, 2020

I see @buzinas, I didn't see that skip: true wouldn't make the query.

And I fully agree, this shouldn't require looking in the the tests, I just wanted to help out those that were hit and running this thread.

@caycecollins
Copy link

caycecollins commented Jul 8, 2020

In my case, I was able to get skip working with a dynamic value (I"m using @apollo/client 3.0.0-rc.10) on a query that has a required variable. It was ignoring skip: true for me as well, but as soon as I added a condition to my required variable value, it behaved as expected:

  const { data: { testList = [] } = {} } = useQuery(
    GET_TEST_LIST,
    {
      skip: !shouldQueryExecute,
      variables: { session: shouldQueryExecute && session },
    }
  )

As soon as I remove the condition for the variable session, it will execute as soon as my component loads with skip still in place (and equal to true).

Hope this helps anybody with a similar use case.

@buzinas
Copy link

buzinas commented Jul 8, 2020

@caycecollins changing the CodeSandbox example to something similar to what you suggested still doesn't work for me.

@caycecollins
Copy link

@buzinas I simplified yours to be closer to mine in this CodeSandbox, but it seems that skip works fine without needing to do anything.

My project has an AuthLink and ErrorLInk setup, and my component has several other query and mutation hooks, uses of useState and useEffect hooks, and other components which query the same data as well... so I'll keep digging but in my specific case, adding a condition to the passed variable session was the only way I was able to get skip to work.

@buzinas
Copy link

buzinas commented Jul 8, 2020

@caycecollins Your example is exactly the opposite of the one that doesn't work. You're starting skip with true and then setting it to false. The bug happens when you start it with false and then set it to true. You also need to send different variables, otherwise – by design – Apollo will just use whatever it has on the cache afterwards.

@caycecollins
Copy link

@buzinas sorry if that was misleading or confusing. The sandbox I made was an attempt to show the issue I was experiencing with my case, but it seems to work fine in CodeSandbox (which means something else is at play in my case).

When the component loads, skip should be true to prevent the query from executing, then later (after another query has finished and the component re-renders) I set skip to false which should (at that time and not before) execute the query.

My issue is the same as the issue author, where skip was ignoring true and executing regardless. I'm still not entirely sure why adding the condition fixes it in my case, but it does.

@lhz516
Copy link

lhz516 commented Jul 12, 2020

I use this to prevent network request:

fetchPolicy: isSkip ? : 'cache-only' : 'cache-and-network'

It's a walk around though. Hope this issue can be fixed.

@willopez
Copy link

This is an issue for us as will, the skip param does not work.

@MichaelBurgess
Copy link

MichaelBurgess commented Jul 16, 2020

Seeing this on @apollo/client 3.0.0 / 3.0.1 on the client side also.

For me it makes no difference if skip is dynamic. If I set skip: true, it will still make the query when the component mounts.

Interestingly as others have noted, if you have no variables, or variables set to {}, then the query is skipped as expected.

@Hamzaalam
Copy link

This is issue persist in the version of "@apollo/react-hooks": "3.1.5"

@3nvi
Copy link

3nvi commented Jul 16, 2020

Can also confirm that we have to rollback to an old Apollo beta because of that. @benjamn or @hwillson is there a chance you can let us know whether we should expect a patch release for this (since an it broke during a beta)?

@bauwaerter
Copy link

A workaround that I am currently using for this is using useEffect with a lazy query.

const [myLazyQuery, { loading, error, data}] = useMyLazyQuery()

useEffect(() => {
    if(var1) {
        myLazyQuery({
            variables: { var1 }
        })
    }
}, [var1, myLazyQuery])

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests