query re runs after a a mutation and cache update

[tafelito]

I've spent a few hours debugging the internals and trying to figure out if this is an actual issue or if it's working as intended. or maybe I'm just doing something wrong.

It's a very simple use case

I have 2 queries and 1 mutation. 1 query gets a list of items. The 2 query get's an item by id. I have a page with a list of items, and a second page with the items details. On the details page, I have the mutation where it updates the item. The result of the mutation could be either the item updated, or null in case the items needs to be removed.
What I'm doing is, checking if the returned data is null, I evict the item from the cache, if not, I let AC to update the cache with the results.
When the results are null, the item of course is not automatically deleted from the cache, so I evict it from the cache in the update function and then from the read fn in the typePolicy I filter the items with canRead.
After I finish the update, I go pack to the list page.

The issue I have is that, after the item is updated in the cache, the item details query is executed again, a networks request is made to the server, and since I request an item by id, I get the result back. The item is then back to the cache. And Not only that, when I navigate after the onComplete to the list page, the query for the items list is also executed an another request is made. The request in this case, brings no results, but since I already have the item on the cache, the list query returns that item.

Is it ok that the item details query re runs after the cache update? is there anyway to prevent that? I tried setting the broadcast property from the cache evict to false but that didn't work and as far as I understand, that is used for a different purpose than this. Please correct me if I'm wrong

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: 84.0.4147.89
    Safari: 13.1.1
  npmPackages:
    @apollo/client: 3.0.2 => 3.0.2 
    @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

[danfernand]

#6760

I created 2 codesandbox examples to showcase this issue!

[benjamn]

If I'm understanding the flow correctly, it sounds like you want those queries to start out with options.fetchPolicy === "cache-first" (the default fetch policy), and then respond to cache updates after the first network request, but not make additional network requests, even if their data becomes incomplete?

If you update to @apollo/[email protected] or later (3.1.2 is the latest version right now), you can take advantage of a new option, options.nextFetchPolicy (see #6712), which will switch fetch policies after the first request:

const { loading, data } = useQuery(THE_QUERY, {
  // This is the default fetch policy, so you don't usually have to specify it,
  // but I wanted to be explicit.
  fetchPolicy: "cache-first",
  // This means: stop making network requests for this query after the first
  // request, but keep listening to the cache. Explicitly refetching still works.
  nextFetchPolicy: "cache-only",
  // Tolerating partial cache data is probably useful if you can't fall back
  // to the network when cache data is incomplete.
  returnPartialData: true,
});

If you don't change the fetch policy with options.nextFetchPolicy, Apollo Client definitely will try to make a network request to refresh evicted/incomplete data, so the behavior you described is working as designed.

[benjamn]

@tafelito Since @danfernand mentioned #6760, you might also want to check out my comment there: #6760 (comment)

[tafelito]

@benjamn thanks for the response and the detailed explanation.

If I'm understanding the flow correctly, it sounds like you want those queries to start out with options.fetchPolicy === "cache-first" (the default fetch policy), and then respond to cache updates after the first network request, but not make additional network requests, even if their data becomes incomplete?

If by incomplete you mean deleted data, yes, that's the correct flow, I don't need to make an additional request if I deleted an item.

I guess the nextPolicy can be useful here and I can make it work (i have to make a few more tests) but to be honest I was looking more for an option no to broadcast a cache update, so If I delete an item, and I say broadcast false, then do nothing (is standby supposed to do that?)

The reason why I need to do that is because my flow is like this

New Item
1. Go to items list
2. Create new item. Goes to item's detail page with item null
3. Save new item. Stays in details page, but shows the item data (need to refresh cache)

In step 3 the details query is not re running, I have to manually re fetch or use the data coming back from the mutation

~~Edit Item ~~
1. Go to items list
2. Click on an item. Goes to item details page
3. Save the item. After it's saves, it's removed from the cache. onComplete takes you back to list page

Because the cache is updating after deleting the item from the cache, in step 3, before navigating back to the list, because the item does not exist anymore and the details query re runs, it shows the details page like I'm creating a new item. (step 2 of new Item flow)
That's why if I had a way of not broadcasting the update when I delete the item from the cache, then I can go the list page without a flickering showing the new items page first

@benjamn I have to keep testing this but nextPolicy seems to be what we need

[tafelito]

@benjamn we've been testing this for the last couple of days and we found one use case where we couldn't find a proper way to use the combination of fetchPolicy and nextFetchPolicy

As ai detailed before, sometimes we want to request an item, and then if we delete the item from the cache, we wan't to prevent a network request. That works if we do what you suggested before, use the fetchPolicy: "cache-first" and nextFetchPolicy: "cache-only"

Now the issue we have, is that sometimes, we need to use nextFetchPolicy: "standby", so we don't listen to cache updates either. The problem is that we didn't find a way to switch btw this 2 policies dynamically depending on the mutation we execute. Ideally, we'd like something like the broadcast when you do update the cache, so when you set it to false, no query should re run.

Isn't that the purpose of doing this?

cache.evict({
   id: cacheId,
   broadcast: false
});

[KeithGillette]

@benjamn wrote:

  // This means: stop making network requests for this query after the first
  // request, but keep listening to the cache. Explicitly refetching still works.
  nextFetchPolicy: "cache-only",

Is that behavior description correct, @benjamn? After setting nextFetchPolicy: 'cache-only' to avoid unnecessary automatic network refetches after a cache eviction, I get a Invariant Violation: cache-only fetchPolicy option should not be used together with query refetch. error when explicitly refetching by including refetchQueries on a mutation.

[gbilodeau]

@benjamn Same question as @KeithGillette . Explicit refetching doesn't work for me, and given the code, I'm not sure why it should?

    ObservableQuery.prototype.refetch = function (variables) {
        var fetchPolicy = this.options.fetchPolicy;
        if (fetchPolicy === 'cache-only') {
            return Promise.reject(process.env.NODE_ENV === "production" ? new InvariantError(12) : new InvariantError('cache-only fetchPolicy option should not be used together with query refetch.'));
        }
        // continued...
    };

[benjamn]

@KeithGillette @gbilodeau Ahh, right, that. My take is that we should remove that invariant. Calling refetch should always cause the query to make a network request, which may involve temporarily changing (or disregarding) the current fetchPolicy. The fetchPolicy should revert back to whatever it was before the refetch.

[benjamn]

@KeithGillette @gbilodeau Ok, I've removed that invariant in @apollo/[email protected], so I believe my previous comment is now correct (for that version of the library).

[gbilodeau]

It seems more in line with my (rather new and untested) mental model. Anyway, looking forward to the release, thanks!

[tafelito]

@benjamn I know the OP is already fixed and so the subsequent related issues posted here, but I'm still having the evict issue mentioned here #6702 (comment). This is also related to the issue #6962 I recently opened. Basically what I'm seeing is that even using broadcast: false when evicting/modifying an item from the cache is still making the queries to re run, no matter what fetchPolicy they have

[smash-96]

@tafelito

This solution is working for me, thanks

[forrestwilkins]

@benjamn Setting both nextFetchPolicy: "cache-only" and returnPartialData: true resolved an issue I was having with unintended network refetches after calling cache.evict. Thanks!