Avoid making network requests when skip is true

[hwillson]

When skip is set to true but other updated options have been passed into useQuery (like updated variables), useQuery will still make a network request, even though it will skip the handling of the response. This commit makes sure useQuery doesn't make unnecessary skip network requests.

Fixes #6670
Fixes #6190
Fixes #6572

[Titozzz]

Great Job, we were just having that issue right now ❤️

[hwillson]

This test was behaving flakily due to its use of MockedProvider's newData function, which is kinda broken. I've adjusted the test to be more specific about the mocked requests it's working with, and added a few quick notes about the full workflow.

[jcreighton]

🎉

[benjamn]

I'm okay with this conservative change/bugfix for 3.1.2, but I would really love to see the previous data delivered at some point, perhaps even in 3.2.0.

[macrozone]

I'm okay with this conservative change/bugfix for 3.1.2, but I would really love to see the previous data delivered at some point, perhaps even in 3.2.0.

[buzinas]

+1

[gtolarc]

I used the logic to create a query by setting skip to true and use refetch only at the desired timing. After this patch, this logic no longer works with following error. Is this the intended situation?

TypeError: Cannot read property 'refetch' of undefined

[gtolarc]

Currently, when the first option starts with skip: true, there is a problem that refetch is not ready and cannot be executed. How about changing the order as follows?

private updateObservableQuery() {
    // If we skipped initially, we may not have yet created the observable
    if (!this.currentObservable) {
      this.initializeObservableQuery();
      return;
    }

    if (this.getOptions().skip) return;

[sirugh]

Not that this isn't a bug, but isn't an initial state with skip: true what lazy query is for?

ie

const [runQuery, response] = useLazyQuery(myQuery);
useEffect(() => {
  if (!skip) {
    runQuery();
  }
}, [skip]);

[gtolarc]

Since there is no clear guidance on this in the official documents, a lot of users will think and use just like me. It is not easy to think of 'skip' affecting 'refetch' without any explanation.

And I can do it as you explained, but it has already become a strangely shaped code. And useLazyQuery's refetch function returns void, so we need to add more useEffect to use the response. The return value of useQuery's refetch is a promise, so you can use data directly with async/await. It's confusing why it was designed this way, but not all of these relationships seem clear and consistent.

[axelvaindal]

I just had exactly the same issue as you @gtolarc and was also using useQuery to get refetch because useLazyQuery returns void instead of a promise.

I'd be in favor of init the observable as well, as this change is quite breaking for a lot of people I guess 😕

[shrugs]

i've confirmed that updating to 3.1.3 in my project solves my skip-related issue

perhaps also solved apollographql/react-apollo#3492

[eugle]

@shrugs Upgrade 3.1.3 does not solve the problem that SKIP is true and still requests the network

[FezVrasta]

Can confirm that this issue is still there in 3.1.4 and 3.2.0

[davidworkman9]

Also still seeing this on 3.2.0.

[FezVrasta]

For anyone having the same problem (everybody), here's a custom useQuery hook I'm using as a drop-in replacement to fix the issue:

import { useEffect } from 'react';
import { useLazyQuery } from '@apollo/client';

// The default Apollo's useQuery hook ignores the `skip` option if it's
// set to true the first time the hook is processed
export function useQuery(query, options) {
  const [execQuery, result] = useLazyQuery(query, options);

  useEffect(() => {
    if (options.skip !== true) {
      execQuery();
    }
  }, [execQuery, options.skip]);

  return result;
}

[bln-engaged]

This custom hook only seems to work if you don't need refetch or other useQuery capabilities.

For anyone having the same problem (everybody), here's a custom useQuery hook I'm using as a drop-in replacement to fix the issue:

import { useEffect } from 'react';
import { useLazyQuery } from '@apollo/client';

// The default Apollo's useQuery hook ignores the `skip` option if it's
// set to true the first time the hook is processed
export function useQuery(query, options) {
  const [execQuery, result] = useLazyQuery(query, options);

  useEffect(() => {
    if (options.skip !== true) {
      execQuery();
    }
  }, [execQuery, options.skip]);

  return result;
}