urql-graphql / urql

The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
https://urql.dev/goto/docs
MIT License
8.65k stars 452 forks source link

cursorPagination causing cache to persist on queries with different args #3255

Closed zcahsja closed 1 year ago

zcahsja commented 1 year ago

Describe the bug

I'm working on a reddit clone and have encountered a problem with my cursorPagination function displaying a cached query's result even when the queries arguments have changed.

The homepage displays a list of posts - clicking on a post navigates the user onto a new page [id].tsx where id is given by the clicked post. This page then uses the id of the post as part of the variables for a comments query (query to fetch all the comments on this post). The issue I am having is that when I click on a post and then navigate back and then click on another post I am seeing the comments corresponding to the first post I clicked on as well as the next post. I believe this is a cache issue as when I refresh the page I correctly see only the comments corresponding to the post I'm viewing.

After trying to debug, I'm pretty sure the custom cursorPagination function (obtained from Ben Awad's reddit clone tutorial) is causing the cache issue as when I comment it out I no longer see this bug where comments belonging to other posts are displayed. This is unexpected as I was under the impression that since my cache keys consist of the query args then as the args change (different postId's) it shouldn't be accessing the result of a cached query with different args.

Below is my cursorPagination function as well as the exchanges passed to my urql client

const cursorPagination = (keyString: string): Resolver => {
  return (_parent, fieldArgs, cache, info) => {
    const { parentKey: entityKey, fieldName } = info;
    const allFields = cache.inspectFields(entityKey);
    const fieldInfos = allFields.filter((fi) => fi.fieldName === fieldName);
    const size = fieldInfos.length;
    if (size === 0) {
      return undefined;
    }
    let hasMore = true;
    const fieldKey = `${fieldName}(${stringifyVariables(fieldArgs)})`;
    const isItInTheCache = cache.resolve(entityKey, fieldKey);
    let cacheBool: Boolean = !!isItInTheCache;
    info.partial = !cacheBool;
    const results: string[] = [];
    fieldInfos.forEach((fi) => {
      const key = cache.resolve(entityKey, fi.fieldKey) as Entity;
      const data = cache.resolve(key, keyString) as string[];
      const _hasMore = cache.resolve(key, "hasMore");
      if (!_hasMore) {
        hasMore = _hasMore as boolean;
      }

      if (data) {
        results.push(...data);
      }
    });
    if (keyString === "posts") {
      return {
        __typename: "PaginatedPosts",
        posts: results,
        hasMore,
      };
    } else if (keyString === "comments") {
      return {
        __typename: "PaginatedComments",
        comments: results,
        hasMore,
      };
    } else if (keyString === "notifications") {
      return {
        __typename: "PaginatedNotifications",
        notifications: results,
        hasMore,
      };
    }
  };
};

const exchanges = [
  dedupExchange,
  cacheExchange({
    keys: {
      PaginatedPosts: () => null,
      PaginatedComments: () => null,
      PaginatedNotifications: () => null,
    },

    resolvers: {
      Query: {
        posts: cursorPagination("posts"), // just a modified simplePagination() function, close to the one on the docs
        notifications: cursorPagination("notifications"),
        comments: cursorPagination("comments"),
      },
    },
    updates: {
      Subscription: {
        //...
      },
      Mutation: {
        //...
      },
    },
  }),
  errorExchange,
];

Reproduction

https://github.com/zcahsja/reddit-clone-with-urql

Urql version

urql 2.2.0

Validations

kitten commented 1 year ago

First of all, you're correct in that each field has basically its own place in the cache, based on the field's name, its arguments, and its parent.

However, resolvers are potentrially much more powerful than that. And here, you've written a resolver that accesses more fields than one.

Any time you call cache.inspectFields, you're getting results from multiple fields. Any time you're calling cache.resolve, you're getting results from a different field.

Before I continue, just a quick note. Issues are not for questions or bugs in the code you write or use, which isn't part of urql. So, just in case you don't know, I'll convert this into a discussion for questions. The issue template calls this out, but just moving this and letting you know so you're aware :v:

The problem here you're having is one with filtering. Your resolver is using this:

    const allFields = cache.inspectFields(entityKey);
    const fieldInfos = allFields.filter((fi) => fi.fieldName === fieldName);

This explicitly says to give you all fields of a name on entityKey. Depending on what this entity is that's every single result that this field ever had with every single combination of arguments.

Compare this to what the simplePagination helper did we've pre-written: https://github.com/urql-graphql/urql/blob/7a11714f02e77fb0b91969b06ee223fbf19164bb/exchanges/graphcache/src/extras/simplePagination.ts#L92-L94

It has additional logic to compare the field arguments and narrow this list of fields down further.


Now, I'd love to send you on your merry way, but I'm afraid, writing complex resolvers like these can be tough, and it's not unlikely you'll run into more issues. This is actually something we addressed with Ben back when he released his video on this.

While there's potentially numerous more ways to do pagination with Graphcache, and not just this one or the built-in ones, and other GraphQL libraries have different built-in methods that could also be replicated in Graphcache, there is a much easier way.

You can write components to combine pages for you: https://formidable.com/open-source/urql/docs/basics/ui-patterns/#infinite-scrolling

This can even be done recursively.

This way, you're moving pagination into the UI and your cache doesn't even have to know that it's infinite pagination