Apollo-client: Updating cache triggers queries when fetchPolicy: cache-and-network is used

[hinok]

This issue doesn't exist on 3.0.2
This issue exists on >3.0.2

Intended outcome:

Update cache without firing network request while using fetchPolicy: cache-and-network.

Actual outcome:

In the past there was a possibility to update cache without triggering network request by using cache.writeData etc.
Now by using cache.modify, query is re-run and network request is fired.

How to reproduce the issue:

Demo: https://codesandbox.io/s/sleepy-clarke-19944?file=/src/AddUser.js

Open https://e3odl.sse.codesandbox.io/ to start the apollo server used in the reproduction.

Important code

Users.js

import React from "react";
import { gql, useQuery } from "@apollo/client";

const ALL_USERS = gql`
  query AllUsers {
    users {
      id
      name
      active
    }
  }
`;

export default function Users() {
  const { loading, error, data } = useQuery(ALL_USERS, {
    fetchPolicy: "cache-and-network"
  });

  if (loading) {
    return <p>Loading…</p>;
  }

  if (error) {
    return <p>Error</p>;
  }

  return (
    <ul>
      {data.users.map((user) => (
        <li key={user.id}>
          {user.active ? user.name : <s>{user.name}</s>}{" "}
          {user.active ? "Active" : "Not active"}
        </li>
      ))}
    </ul>
  );
}

AddUser.js

import React, { useRef } from "react";
import { gql, useMutation } from "@apollo/client";

const ADD_USER = gql`
  mutation AddUser($user: UserInput!) {
    updateUser(user: $user) {
      id
      name
      active
    }
  }
`;

export default function AddUser() {
  const [mutate, { loading, error }] = useMutation(ADD_USER);
  const nameRef = useRef();
  const activeRef = useRef();

  const handleClick = () => {
    const name = nameRef.current.value;
    const active = activeRef.current.checked;

    mutate({
      variables: {
        user: {
          name,
          active
        }
      },
      optimisticResponse: {
        id: Math.random(),
        name,
        active
      },
      update(cache, { data: { updateUser } }) {
        cache.modify({
          broadcast: false,
          optimistic: false,
          fields: {
            users(existingUsers = []) {
              const newUserRef = cache.writeFragment({
                data: updateUser,
                fragment: gql`
                  fragment NewUser on User {
                    id
                    name
                    active
                  }
                `
              });
              return [newUserRef, ...existingUsers];
            }
          }
        });
      }
    });
  };

  if (error) {
    return <p>Could not add user</p>;
  }

  return (
    <div>
      <div>
        <input ref={nameRef} type="text" name="name" placeholder="Name" />
      </div>
      <div>
        <label>
          <input ref={activeRef} type="checkbox" name="active" />
          Active
        </label>
      </div>
      <div>
        <button disabled={loading} onClick={handleClick}>
          {loading ? "Adding..." : "Add user"}
        </button>
      </div>
    </div>
  );
}

Versions

@apollo/client 3.1.3

[sirugh]

I think I'm seeing a similar behavior when using the merge type policy function. I have a mutation that removes an item from an array and returns the state of that array in the mutation response, but a query observing the array is being re-fired after the cache is updated with the mutation response.

[hinok]

Based on the comment https://github.com/apollographql/apollo-client/issues/6760#issuecomment-668188727

It seems that setting nextFetchPolicy: "cache-first" resolves the issue.

[sirugh]

That fixed it for us! At least half :D We still are seeing some queries being re-fired when others resolve that affect the cache, even after applying the nextFetchPolicy. I think this has to do with a misconfigured merge function.

Edit: I'm not really able to repro this anymore, but I've been working on our merge functions lately and I think I might have "solved" it accidentally. If it happens again I'll try to come back and update this comment, or open a new issue with repro.

[benjamn]

@hinok I see you edited your comment, but I was curious what you meant by "unfortunately optimisticResponse doesn't work in this case." Did you mean there's no way to set a nextFetchPolicy for the optimistic part, or that using nextFetchPolicy makes optimistic updates behave differently, or something else?

To your point about the difference in broadcasting behavior between cache.writeQuery and client.writeQuery in AC2, you can call cache.writeQuery({ ..., broadcast: false }) in AC3 to write data without broadcasting those changes.

[hinok]

@benjamn I edited my comment because I had a small issue on my side in optimisticResponse part of code. I missed __typename if I remember correctly and initially I thought that it's a new behaviour in ac3 or just a bug 😅 . It wasn't bug, it was my fault so ac3 works correctly therefore I edited my comment...

btw it'd be nice to have a warning or info about optimisticResponse that doesn't work because some fields are missing etc.
I'm not sure what broadcast, optimistic options do in cache.modify. I don't see any difference in my demo after setting true/false. Good code examples, demos would be super helpful to understand and explain some of the "new" features of ac3.

If nextFetchPolicy is the official way to solve the problem, I think we can close this issue or we can focus on @sirugh problem.

[benjamn]

Totally agree that we need more examples of all this stuff!

Although nextFetchPolicy is the official solution for now, we are also planning to make some improvements to nextFetchPolicy soon, so that it can optionally be a function that receives the current policy, which I think will make it safer to use in your defaultOptions, so you could reenable the fallback behavior by default across your application, if that's what you prefer.

[benjamn]

@hinok @sirugh With @apollo/[email protected] (which includes #6893), it should be possible to configure nextFetchPolicy globally, using a function instead of just a string:

new ApolloClient({
  link,
  cache,
  defaultOptions: {
    watchQuery: {
      nextFetchPolicy(lastFetchPolicy) {
        if (lastFetchPolicy === "cache-and-network" ||
            lastFetchPolicy === "network-only") {
          return "cache-first";
        }
        return lastFetchPolicy;
      },
    },
  },
})

It's important to use a function here, so you're only modifying cache-and-network and network-only, not any other fetch policies. It would be weird/wrong to change cache-only to cache-first, for example.

Thanks in advance for testing out this new functionality and reporting any problems you encounter!