apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.28k stars 2.64k forks source link

Cache data not evicted after returning INVALIDATE in modify #9319

Open lprokein opened 2 years ago

lprokein commented 2 years ago

I have a query that returns list of projects created from a specific template. Query has a variable templateId - so for which template projects should be returned.

I have a mutation for creating new projects.

After project is created though mutation I want to invalidate the cache for listing projects of the template which was used for creating this last project.

So I use cache.modify and return INVALIDATE for relevant item.

But this does not invalidate the cache and there is no refetch next time calling query for listing projects of template.

I tried adding broadcast: false and cache.gc(). No help.

But when I return DELETE instead of INVALIDATE cache is deleted and refetch is called next time calling query for listing projects of template.

So whats the point of INVALIDATE then? My understading was that INVALIDATE should be used for this use-case - based on reading docs and other issues. Am I wrong and should DELETE be used instead ?

Intended outcome: When returning INVALIDATE, data should be invalidated and refetched next time.

Actual outcome: Data is stale and no refetch is initiated.

How to reproduce the issue:

export const useCreateProject = () => {
  const [mutate] = useMutation<CreateProject, CreateProjectVariables>(
    CREATE_PROJECT,
  );

  const updatedMutate = useCallback(
    (variables: CreateProjectVariables, callback?: () => void) => {
      mutate({
        variables,
        update: (cache, {data}) => {
          const {templateId} = data.createProject.project;

          cache.modify({
            broadcast: false,
            fields: {
              templateProjects: (cachedValue, {storeFieldName, INVALIDATE}) => {
                console.log(storeFieldName);

                if (storeFieldName.startsWith("templateProjects")) {
                  const jsonVariables = storeFieldName.substring(
                    storeFieldName.indexOf("{"),
                    storeFieldName.lastIndexOf("}") + 1,
                  );

                  const variables = JSON.parse(jsonVariables);
                  if (variables.templateId === templateId) {
                    console.log("INVALIDATING: ", cachedValue);
                    return INVALIDATE;
                  }
                }

                return cachedValue;
              },
            },
          });

          cache.gc();
        },
      });
    },
    [mutate],
  );

  return {createProject: updatedMutate};
};

Versions

System:
    OS: macOS 12.1
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.1/bin/yarn
    npm: 8.3.0 - ~/.nvm/versions/node/v16.13.1/bin/npm
  Browsers:
    Chrome: 97.0.4692.71
    Firefox: 95.0.2
    Safari: 15.2
  npmPackages:
    @apollo/client: 3.5.7 => 3.5.7 
    apollo-link-error: ^1.1.13 => 1.1.13 
  npmGlobalPackages:
    apollo: 2.33.9
benjamn commented 2 years ago

@lprokein You can read more about the somewhat obscure role INVALIDATE plays in the docs for client.refetchQueries, which allows you to refetch invalidated queries without actually removing any data from the cache.

See especially this note:

Before client.refetchQueries was introduced, the INVALIDATE sentinel was not very useful, because invalidated queries with fetchPolicy: "cache-first" would typically re-read unchanged results, and therefore decide not to perform a network request. The client.refetchQueries method makes this invalidation system more accessible to application code, so you can control the refetching behavior of invalidated queries.

If you don't mind removing/evicting the data, DELETE is the easier/better tool. The only use case for INVALIDATE is refetching without first evicting the data.

lprokein commented 2 years ago

@benjamn thanks for your comments. I am still a bit confused what's the actuall purpose of INVALIDATE but at least I now now know it's not doing what I expected.

I check the issue that you linked I kind of agree with this comment https://github.com/apollographql/apollo-client/issues/7060#issuecomment-942563832 that I kind of miss a way how to invalidate specific queries after mutation the way that:

Please correct me if there is a way how to achieve this.

Is there a way I can read whether the cached query is active/mounted or not so I can decide wheter to use DELETE or INVALIDATE ?

I know client.refetchQueries has active and all options but I don't see a way how to use that to target active-only and non-active-only. As the all includes all.

jbmikk commented 9 months ago

@benjamn I think the only reason INVALIDATE has an obscure role is because it's hard to use, but that doesn't mean it's not necessary. I think being able to render stale data while it is being refetched is very important for interactive applications.

I know maybe CRUD applications maybe aren't Apollo's main type of application, but Apollo still supports mutations, so having a way of dealing with stale data across queries in the application is important, and dealing with it in a visually appealing way is important too. There's no reason to evict data if that data is the best you have to show at the moment.

I think this feature is so important that having to implement the refetchQueries boilerplate in each application just to fetch all active stale queries deserves its own cache policy (maybe something like valid-cache-first).

Even more so if you consider what happens when inactive queries become active, if they contain data flagged invalid, those should be refetched too and calling refetchQueries after a mutation can't deal with that case.

mogelbrod commented 3 months ago

I'm trying to make apollo client invalidate some paginated fields (x and y below) in response to receiving an updated value of another field (entityIds below).

the GraphQL schema looks like something like the following:

type List {
  id: ID!
  entityIds: [ID!]
  x: XRelayConnection # { edges { node: X }, ... }
  y: YRelayConnection # { edges { node: Y }, ... }
}
subscription listUpdate(list: ID!) {
  list: List # { id entityIds } fetched
}
mutation addToLibrary(list: ID!, entity: ID!) {
  list: List # { id entityIds } fetched
}
mutation removeFromLibrary(list: ID!, entity: ID!) {
  list: List # { id entityIds } fetched
}

The subscription is maintained during the app lifecycle to ensure the internal entityIds array is kept up to date when changes to it is made by other clients/users. entityIds are the ids for items in x and y, which we track separately as those are paginated and therefore unfeasible to recursively fetch all ids on every single update to entityIds. I opted to use type policies to invalidate x and y on changes to entityIds, but the inability to use INVALIDATE to mark those fields as stale makes this approach seem impossible. The DELETE workaround is not really viable as I still want to display stale data for x/y while refetching in the background. refetchQueries() only appears to refetch queries that are active, which won't be the case if the user is not currently on the page showing x/y data. It's also not immediately available to the apollo cache.

typePolicies.Library.fields.entityIds = {
  merge(existing: string[] | undefined, incoming: string[], c) {
    // Hack to access the parent `library` field, see side note
    const libraryId: string | undefined = c.variables
      ? c.variables.libraryId || c.variables.id
      : undefined
    if (!libraryId) {
      throw new Error('Must include one of `id`/`libraryId` as variable when fetching `MusicLibrary.ids`')
    }
    if (existing && incoming) {
      const shouldInvalidate = { X: false, Y: false }
      const remainingIds = new Map(existing.map((id) => [id, false]))
      const invalidateTypeFor = (id: string): boolean => {
        const type = parseId(id).__typename // we can infer typename from ID for this use case
        if (type && type in shouldInvalidate && !shouldInvalidate[type]) {
          shouldInvalidate[type] = true
        }
        return shouldInvalidate.X && shouldInvalidate.Y
      }
      // Invalidate due to new additions?
      for (const id of incoming) {
        if (remainingIds.has(id)) {
          remainingIds.set(id, true)
        } else if (invalidateTypeFor(id)) {
          break // New ID encountered - invalidate corresponding type
        }
      }
      // Invalidate due to removals?
      for (const [id, wasKept] of remainingIds) {
        if (!wasKept && invalidateTypeFor(id)) {
          break
        }
      }
      c.cache.modify({
        id: c.cache.identify({ __typename: 'Library', id: libraryId }),
        fields: {
          x: (cached, ctx) => {
            return shouldInvalidate.X ? ctx.INVALIDATE : cached
          },
          y: (cached, ctx) => {
            return shouldInvalidate.Y ? ctx.INVALIDATE : cached
          },
        },
      })
    }
    return incoming
  },
}

side note: The merge function has to be set on entityIds instead of directly on Library due to this issue: https://github.com/apollographql/apollo-client/issues/11221