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

watchQuery over-fetching #7114

Open
klemenoslaj opened this issue Oct 2, 2020 · 10 comments
Open

watchQuery over-fetching #7114

klemenoslaj opened this issue Oct 2, 2020 · 10 comments

Comments

@klemenoslaj
Copy link

Intended outcome:

When having active watchQuery in one component and triggering another watchQuery deeper in the component tree only the one deeper in the tree should create a new network request.

The 1st query should not be triggered together with the 2nd query. If any field shared between the two queries changes with the response of 2nd query, the 1st watchQuery should update due to cache changes anyway.

Example

1st query

query {
  file(id: 1000) {
    id
    name
    size
  }
}

2nd query

query {
  file(id: 1000) {
    id
    name
    mimetype
  }
}

Actual outcome:

When the 2nd query is executed, the 1st query is sending a new network request again effectively over-fetching.

How to reproduce the issue:

Reproduction: https://stackblitz.com/edit/simple-apollo-angular-example-maz5cx
Keep refreshing with the stackblitz refresh button and more often than not (gif below).

apollo-over-fetching

Versions

  System:
    OS: macOS 10.15.7
  Binaries:
    Node: 12.18.2 - ~/.nvm/versions/node/v12.18.2/bin/node
    Yarn: 1.22.5 - ~/Projects/censhare-hub/node_modules/.bin/yarn
    npm: 6.14.6 - ~/.nvm/versions/node/v12.18.2/bin/npm
  Browsers:
    Chrome: 85.0.4183.121
    Edge: 85.0.564.68
    Firefox: 81.0
    Safari: 14.0
  npmPackages:
    @apollo/client: ^3.0.0 => 3.2.2 
    @apollo/link-persisted-queries: ^1.0.0-beta.0 => 1.0.0-beta.0 
    apollo: ^2.30.3 => 2.31.0 
    apollo-angular: ^2.0.4 => 2.0.4 
@KeithGillette
Copy link
Contributor

I believe I am encountering the same issue in our Apollo Client 3.2.2 project. In our case, we have 2 sibling components, each initiating a watchQuery on queries requesting different fields on the same object. The query document, operation name, and variables are distinct. Activating a second component starts its watchQuery, which then triggers an update on the watchQuery in the first component. That first query differs in variables from the first in using pagination for the requested field, and so is refetched with incorrect pagination since it's triggered outside of a fetchMore call, resulting in incorrectly displayed results.

@CaptainHansen
Copy link

CaptainHansen commented Oct 21, 2020

Pretty sure I'm having the same issue as well. We use fetchMore for pagination with the no-cache fetch policy on a Query component in our react webapps. So far, we're using apollo client v3.2.1 in production for one internal-use webapp, and I've updated it
to the latest version in dev to test further (v3.2.5) to see if that would solve the issue, but the issue persists. In our scenario, results in dev and prod differ.

In dev, our webapp still function properly, however, after fetchMore is called, it seems the reobserve pathway in apollo client is causing the original query to be fired off again simply resulting in unnecessary network traffic.

In production, both the correct and unnecessary query are still being fired off, however, the paginated results only appear on the screen for a split second and then vanish making apollo client unusable for further production deployment until either a workaround is found or a fix is pushed.

The differences between dev and prod may be explained due to significantly increased latency in dev (issues in production may be the result of a race condition) although I haven't had the time to drill down to see why results show as expected in dev and disappear in production.

I spent some time in the chrome debugger and found the extra unnecessary query is coming from https://github.com/apollographql/apollo-client/blob/main/src/core/Reobserver.ts#L52 . I've also found that network-only and cache-and-network fetch policies result in the same behavior in dev, however, the extra fetch call disappears if I change the cache policy to cache-first (however, obviously the initial result set will not be reloaded if a query variables are modified and then cleared which will not work for our apps).

One workaround I've thought of is setting the initial fetch to return 0 results, set the Query component's fetch policy to cache-only, and force fetchMore to be called immediately after loading. Not ideal, but may work. A fix for this would be much appreciated though :) .

@CaptainHansen
Copy link

In case anyone else is trying to figure out how to get around this problem, I can confirm that the workaround I mentioned above works.

We've had so many issues with apollo's cache and endless scroll that we've completely circumvented apollo's cache on any page that uses pagination/endless scroll and are instead using component state in our endless scroll component.

@benjamn
Copy link
Member

benjamn commented Oct 21, 2020

@CaptainHansen I would love your critical feedback on this documentation-in-progress about AC3 pagination: #7175 (preview).

You might also try using nextFetchPolicy to switch back from network-only or cache-and-network to cache-first, after the initial request: #6712.

@benjamn
Copy link
Member

benjamn commented Oct 21, 2020

@klemenoslaj In your reproduction, because you're sending your justIds query over the network, you end up mixing post objects that just have ids together in the same list (Query.posts) with the allPosts objects, which results in missing fields when the allPosts query tries to read those id-only posts, which results in the second network request.

You can fix this by giving your justIds query a cache-only fetch policy, or by annotating its posts field with @client:

    this.apollo.watchQuery<Query>({
      fetchPolicy: 'cache-only',
      query: gql`
        query justIds {
          posts @client {
            id
          }
        }
      `,
    })

@klemenoslaj
Copy link
Author

klemenoslaj commented Oct 22, 2020

@benjamn thanks for the reply.
That might solve the problem in my reproduction, but by no means is that a real world scenario.

My point is, what if I genuinely want to go trough the network for both requests? I hope the reproduced behavior is not as expected.

@benjamn
Copy link
Member

benjamn commented Oct 22, 2020

@klemenoslaj This behavior hinges on your answer to a question that the cache cannot answer for you: is there just one list of Query.posts, with different views for different queries, or should the Query.posts lists corresponding to the different queries remain separate in the cache?

In the first case, since one query requests fewer fields for each post than the other query, the other query will find the post objects from the justIds query insufficient for its purposes, and will probably need to make another request to fill in the missing fields.* This is normal and expected, though you could use a shared fragment to make both queries request the same fields, so results from either query satisfy the other query, though that implies some amount of (necessary) over-fetching.

* This follow-up request could be more efficient, if there was a way to request arbitrary fields for individual entity objects, but that's not a built-in feature of GraphQL, so Apollo Client has to work without that assumption.

In the second case, you now have two independent copies of what is arguably the same data, so you have to worry a bit more about consistency between those two related-but-separate lists of data. If you're interested in this approach, configuring a custom keyArgs function for the Query.posts field should allow you to maintain two separate posts:<extra identifying info> fields within the root query object. Happy to go into more details about that if you like.

@klemenoslaj
Copy link
Author

In the first case, since one query requests fewer fields for each post than the other query, the other query will find the post objects from the justIds query insufficient for its purposes

Shouldn't it be the other way around?

allPosts is the query with all the fieds.
justIds is query with ids only.

allPosts query is fired first, it get's all the ids and the rest of the properties.
Somewhere around that time app-test is rendered which causes justIds query to fire.

I would expect this to be the end. However for some reason the 3rd request is made with the allPosts query.

What you're saying is this (if I got it right, otherwise sorry 😇 ):

justIds is fired - no data in cache - request made
allPosts is fired - no sufficient data in cache - request made

What I am saying is this:

allPosts is fired - no data in cache - request made
justIds is fired - data is in cache, but cache-and-network policy is configured - request made
allPosts is fired again - I expect this should not have happened

What I don't understand is why the allPosts get's fired again? There is absolutely no need for it, is it?
The justIds query is asking for subset of data, but it only goes trough the network because of cache-and-network fetch policy.

@ruanlinos
Copy link

same issue here with fetchMore.

@littlespice33
Copy link

littlespice33 commented Jan 26, 2022

Also experiencing this issue with apollo 3.0.0

In our case we have 2 sibling components calling watchQuery with the same fetchPolicy and operationName and GQL Query object, but with different variables (some variables are the same, some are different).

Every time one sibling component subscribes to a watchQuery, all other siblings fire a request to the network API. Considering that we have a use case to have many siblings in use at once and some of our users do not have very powerful machines, these multitudes of API calls slow down their machines to a large extent, rendering our app almost unusable

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

No branches or pull requests

7 participants