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

Add support for DocumentNode to cache.evict() #440

Open
ValentinH opened this issue Jun 21, 2024 · 2 comments
Open

Add support for DocumentNode to cache.evict() #440

ValentinH opened this issue Jun 21, 2024 · 2 comments
Labels
📕 cache Feature requests related to the cache core Feature requests related to core functionality

Comments

@ValentinH
Copy link

ValentinH commented Jun 21, 2024

First, the issue I'm trying to address with this feature request was perfectly stated in 2016 in this comment:
image

Today, when we have a mutation that is affecting the result of a query done on another page, I'm using one of these approach:

  • setting fetchPolicy to cache-and-network: not ideal because you are over-fetching most of the time
  • using refetchQueries on the mutation: not ideal either for the reasons mentioned in Revisit refetchQueries API #419 + because when using different set of variables (like when doing pagination), it only refetched that most recently used set of variables. In addition, sometimes we just don't want to refetch because the user might not go back to the query page so it could also be over-fetching.
  • using updateQuery from the query and manually update the cache: most performant way to do it but it sometimes super hard or even impossible if the query is reading from an aggregation table for example.

What I'm looking for is a way to invalidate a set of queries similar to react-query invalidateQueries.

I started to use cache.evict({ fieldName: 'example' }) but the issue is that the fieldName isn't type-safe. Therefore, I built this helper function to do this in a safe way from an array of DocumentNode (generated by graphql-codegen):

const invalidateQueries = (cache: ApolloCache<any>, documentNodes: DocumentNode[]) => {
  const fieldsToInvalidate = documentNodes.flatMap((documentNode) =>
    documentNode.definitions.flatMap((definition) => {
      if (!('selectionSet' in definition)) {
        return [];
      }
      return definition.selectionSet.selections
        .map((selection) => {
          if ('name' in selection) {
            return selection.name.value;
          }
          return null;
        })
        .filter(isPresent);
    })
  );
  fieldsToInvalidate.forEach((fieldName) => {
    cache.evict({
      fieldName,
    });
  });
  // I'm not yet sure if I should call `cache.gc()` here 
};

I can then use it like this:

  const [exampleMutation, result] = useMutation(ExampleMutationDocument);

  const doMutation = async () => {
    await exampleMutation({
      variables: {
        value: 'whatever'
      },
      update: (cache) => {
        invalidateQueries(cache, [ExampleQueryDocument]);
      },
    });
  };

This is working fine on the tests I did but now I'm wondering if cache.evict() could directly accept a DocumentNode (or even an array of nodes) to evict all the fields selected by the queries (this comment was also suggesting this).

Basically being able to do:

  const [exampleMutation, result] = useMutation(ExampleMutationDocument);

  const doMutation = async () => {
    await exampleMutation({
      variables: {
        value: 'whatever'
      },
      update: (cache) => {
        cache.evict(ExampleQueryDocument)
      },
    });
  };

or to be even closer to react-query:

 const [exampleMutation, result] = useMutation(ExampleMutationDocument);

  const doMutation = async () => {
    await exampleMutation({
      variables: {
        value: 'whatever'
      },
      invalidateQueries: [ExampleQueryDocument]
    });
  };
@jerelmiller
Copy link
Member

Hey @ValentinH 👋

Thanks for opening the request and thanks for your patience!

This is an interesting idea, though I'd like to push back a little bit on the idea of a document-based solution. Since InMemoryCache is a normalized cache, there are scenarios that get tricky to handle. For example, what happens if you issue 2 queries that request the same field and you only invalidate one of the queries? Does it partially invalidate the other query? Should it refetch the other query because its partially invalidated? Those are things to think through as well.

I do agree with you that there should be a more robust invalidation API, but I'd prefer to use it on the field-level which fits the normalized cache paradigm more closely. In this case, invalidating a field would mark it as stale and any queries that consume that field would refetch in order to synchronize it with the server. Ideally this becomes the replacement for refetchQueries in the future.

On the TypeScript end, I'd like to see InMemoryCache accept a Schema generic type that has full knowledge of your GraphQL schema types in order to better type APIs such as type policies, cache eviction, etc. We are hoping to get something like this either in 4.0 or a minor version shortly after. I'm hoping this would alleviate some of the type safety issues that currently exist when using the cache.

@jerelmiller jerelmiller added core Feature requests related to core functionality 📕 cache Feature requests related to the cache labels Nov 7, 2024
@ValentinH
Copy link
Author

Thanks for the detailed answer.

I like the idea of field level invalidation, I'm looking forward to hearing more about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📕 cache Feature requests related to the cache core Feature requests related to core functionality
Projects
None yet
Development

No branches or pull requests

2 participants