NerdWalletOSS / apollo-cache-policies

An extension of the Apollo 3 cache with support for advanced cache policies.
Apache License 2.0
157 stars 21 forks source link

Question: Root Query, Pagination and Entity TTL's #55

Closed jwarykowski closed 2 years ago

jwarykowski commented 2 years ago

Hey @danReynolds,

I'm doing some experimenting and working with an entity (Contact) which has a TTL of 5 seconds (Its only this low value at the moment to make sure I understand how all the underlying functionality works).

The list page will only fetch 25 items at a time and is paginated by a nextToken. If I let the first 25 references TTL expire and then paginate to the next page by scrolling, once the request is returned and the response written to the cache all the existing references from the initial request have their TTL's become active again, I was expecting that these would remain expired given they haven't been rewritten into the cache as they already exists?

https://user-images.githubusercontent.com/406799/175798375-100a0cfd-d83b-4557-9964-92817b4b4762.mov

Note I'm using the fetchMore updateQuery callback to update my root query getContacts, you can see this below:

export const useGetContacts = (filter: any) => {
  const { data: settings } = useQuery(GET_ACCESS_TOKEN);

  const variables = { ...filter, limit: 25 };

  const { data, error, loading, fetchMore, refetch } = useQuery(GET_CONTACTS, {
    context: { accessToken: settings?.accessToken },
    returnPartialData: true,
    variables,
  });

  const items = data?.getContacts?.contacts || [];
  const nextToken = data?.getContacts?.nextToken;

  return {
    error,
    fetchMore: () =>
      fetchMore({
        updateQuery: (previousQueryResult, { fetchMoreResult }) => ({
          getContacts: {
            contacts: [
              ...(previousQueryResult?.getContacts?.contacts || []),
              ...(fetchMoreResult.getContacts.contacts || []),
            ],
            nextToken: fetchMoreResult.getContacts.nextToken,
          },
        }),
        variables: { nextToken },
      }),
    hasMore: Boolean(nextToken),
    items,
    loading,
    refetch,
  };
};

Should the TTL be reset back to active if the reference already exists in the cache and the TTL has already expired?

I'm wondering whether spreading the previous query results causes this problem 🤔 I would have thought that Apollo knows the reference already exists so this doesn't get rewritten but perhaps there is no check here. Note I don't use a custom query type policy merge function here so perhaps if I changed the implementation and used the mergeObjects helper this might work?

If you have any knowledge around this area that would be greatly appreciated.

danReynolds commented 2 years ago

Yea I think since you spread it then they are now being rewritten and will get their TTL cache time updated. I can look into that more sometime

jwarykowski commented 2 years ago

Hey @danReynolds, thanks, I'll have a deep looker and try merging objects from the type policies to see whether this changes the behaviour.

danReynolds commented 2 years ago

You can also specify a renewal policy to indicate how cache TTLs should be reset: https://github.com/NerdWalletOSS/apollo-cache-policies#renewal-policies. Setting it to none would solve your issue but you may want it reset if it was being written in other ways. Maybe a renewal policy that only resets it if the object has changed could be a potential new feature, although I'm not the biggest fan of deep equality checks.

jwarykowski commented 2 years ago

Hey @danReynolds, using the fetchMore updateQuery callback is in fact deprecated after doing some further research. After converting this to using a type policy with a custom merge function this now works as expected e.g:

getContacts: {
    merge(existing, incoming, { readField, mergeObjects }) {
      const existingContacts = existing?.contacts || [];
      const incomingContacts = incoming?.contacts || [];

      const mergedContacts: any[] = existingContacts
        ? existingContacts.slice(0)
        : [];

      const idIndex: Record<string, number> = Object.create(null);

      if (existingContacts) {
        existingContacts.forEach((contact: any, index: number) => {
          idIndex[readField<string>("id", contact) as string] = index;
        });
      }

      incomingContacts.forEach((contact: any) => {
        const id = readField<string>("id", contact) as string;
        const index = idIndex[id];
        if (typeof index === "number") {
          // Merge the new contact data with the existing contact data.
          mergedContacts[index] = mergeObjects(
            mergedContacts[index],
            contact
          );
        } else {
          // First time we've seen this contact in this array.
          idIndex[id] = mergedContacts.length;
          mergedContacts.push(contact);
        }
      });

      return {
        ...incoming,
        contacts: mergedContacts,
      };
    },
  },
},

I think this ticket can be closed but please add any finding you may have.

danReynolds commented 2 years ago

Ah interesting, okay yea I saw close it out then.