Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC v3] - query still requesting the server even when using skip (sometimes) #6572

Closed
tafelito opened this issue Jul 10, 2020 · 13 comments · Fixed by #6752
Closed

[AC v3] - query still requesting the server even when using skip (sometimes) #6572

tafelito opened this issue Jul 10, 2020 · 13 comments · Fixed by #6752
Assignees
Milestone

Comments

@tafelito
Copy link

I have a werid issue when using skip.

I have a query that is called only when a certain condition is met. This is how I'm doing the query

const { data: subAreasData, loading: subAreasLoading } = useSubareasQuery({
    variables: {
      where: {
        areaId: { _eq: area?.id },
        companyId: { _eq: company?.id },
      },
    },
    skip: !area || !company,
  });

If I do that, the query is still being made. Even If I force the skip to true. useSubareasQuery is actually called twice because the values of the variables changes.

The first time, the value of the variables are this

image

Then one of the variables changes it's value, but the skip condition still true

image

 if (!ref.current || !equal(key, ref.current.key)) { <--- HERE
    ref.current = { key: key, value: memoFn() };
}

The key is actually not equal to ref.current.key

If I instead of doing the above I do this

const skip = !area || !company;
  const subAreasFilters = !skip
    ? {
        areaId: { _eq: area?.id },
        companyId: { _eq: company?.id },
      }
    : {};
  const { data: subAreasData, loading: subAreasLoading } = useSubareasQuery({
    variables: {
      where: subAreasFilters,
    },
    skip,
  });

Then it does actually works and skips the network request.

Should changing the underlying variables object affect the if the request being made?

Any ideas what could the issue be?

Versions

System:
    OS: macOS 10.15.5
  Binaries:
    Node: 13.12.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
  Browsers:
    Chrome: 83.0.4103.116
    Safari: 13.1.1
  npmPackages:
    @apollo/client: 3.0.0-rc.12 => 3.0.0-rc.12 
    @apollo/link-context: ^2.0.0-beta.3 => 2.0.0-beta.3 
    @apollo/link-error: ^2.0.0-beta.3 => 2.0.0-beta.3 
    apollo-upload-client: ^13.0.0 => 13.0.0 
  npmGlobalPackages:
    apollo-codegen: 0.20.2
@yngwi
Copy link

yngwi commented Jul 11, 2020

It sounds a little bit similar to issue #6507 that I reported some time ago. There seems to be something really strange going on.

@tafelito
Copy link
Author

@yngwi yes I think they might be related. I think there are 2 different things happening. As I pointed out, the first issue (my not be an issue) is that the first time the code runs, it actually skipped the request, but as soon as the variables changed, even tho the skip it still true, this !equal(key, ref.current.key)) becomes true and the memoFn is executed. That shouldn't be an issue if the skip was not ignored, and that's the 2 part of the issue. I couldn't find anywhere where the skip is actually used

@KamalAman
Copy link

This issue is also referenced in 6190. The suggested hack usually works. A very bad bug that keeps breaking many things and destroying the dependability of the library.

@benjamn benjamn added this to the Release 3.0 milestone Jul 15, 2020
@hwillson
Copy link
Member

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

@tafelito
Copy link
Author

@hwillson I just tested this with @apollo/[email protected] and the issue is still there. I think the issue that was fixed on #6589 is quite different than I reported here.
The skip is ignored when the useQuery variables change, even if the skip is still true

Can we please re open this?

@gztomas
Copy link

gztomas commented Jul 17, 2020

@hwillson I'm seeing the same issue. This is with @apollo/[email protected]
image
See how fetchQueryObservable is called even with skip: true. I don't see any check for skip downstream after that.
ObservableQuery.prototype.setOptions is causing a fetch even though those options have skip: true.

Sharing more of what I'm seeing so you can repro:
This equality check is evaluating to false

if (
!equal(
newObservableQueryOptions,
this.previousData.observableQueryOptions
)
) {
this.previousData.observableQueryOptions = newObservableQueryOptions;
this.currentObservable
.setOptions(newObservableQueryOptions)
// The error will be passed to the child container, so we don't
// need to log it here. We could conceivably log something if
// an option was set. OTOH we don't log errors w/ the original
// query. See https://github.com/apollostack/react-apollo/issues/404
.catch(() => {});
}
}

And this is how the options are changing
image

I am skipping once the customerId is undefined, but useQuery fetches with the undefined variable anyways.

@sirugh
Copy link

sirugh commented Jul 21, 2020

We're running into this as well but with null variables.

Is it possible to work around this issue by using lazy query in an effect?
I saw another thread with a suggestion to use fetchPolicy: skip ? 'cache-only' : YOUR_POLICY, which seems to work but it's pretty ugly.

@tafelito
Copy link
Author

@sirugh A workaround is to create the variables outside of the useQuery. Check my OP

@sirugh
Copy link

sirugh commented Jul 21, 2020

Creating the skip argument outside of the object did not work nor did using useMemo to memoize it. I am using a cache-and-network fetch policy which probably has something to do with it.

@gandhiamarnadh
Copy link

gandhiamarnadh commented Jul 29, 2020

@hwillson the issue still exists (when variables changes) , please re-open this

V 3.1.0

@leonardoanalista
Copy link

I am on APC 3.0.2 and I can confirm this is still a problem @hwillson @benjamn

That was the PR: #6589

@adrienharnay
Copy link

adrienharnay commented Jul 31, 2020

@apollo/client 3.1.1, issue still occurs. Could we reopen this issue? Or should we open a new one?

@3nvi
Copy link

3nvi commented Jul 31, 2020

@apollo/client 3.1.1, issue still occurs. Could we reopen this issue? Or should we open a new one?

Track it here #6670 (comment)

@hwillson hwillson reopened this Jul 31, 2020
hwillson added a commit that referenced this issue Jul 31, 2020
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
hwillson added a commit that referenced this issue Jul 31, 2020
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
hwillson added a commit that referenced this issue Jul 31, 2020
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
hwillson added a commit that referenced this issue Jul 31, 2020
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
hwillson added a commit that referenced this issue Aug 1, 2020
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
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.