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

Problem with custom type policy for query caching #11968

Open
IgorNovikov10 opened this issue Jul 22, 2024 · 19 comments
Open

Problem with custom type policy for query caching #11968

IgorNovikov10 opened this issue Jul 22, 2024 · 19 comments

Comments

@IgorNovikov10
Copy link

Issue Description

I have a NextJS application that can have the same routes for different domains/locales for example
http://site.com/test-page and http://site.se/test-page. We send a request to the current route to get the page data in this form

const { req, res, resolvedUrl } = context;
const xSite = req.headers["x-site"] as string;

apolloClient.query({
query: GetPageDocument,
variables: {
path: resolvedUrl,
xSite,
}
})

In addition, we send a xSite header in the request so that the content system determines for which locale what data needs to be ssent. The problem is that if there is a page that exists on 2 domains with the same route , for example, http://site.com/test-page and http://site.se/test-page then sometimes the data that is in the cache is mixed and it happens that on one site http://site.com I see data from another http://site.se/test-page and vice versa

Also I tried to setup custom caching for the field that this query returns like this:

typePolicies: {
...
Query: {
        fields: {
          page: {
            keyArgs: ["path", "xSite"],
            read(existing: any, { args, variables }: any) {
              const { path } = args;
              const xSite = variables?.xSite;
              if (!xSite) return undefined;

              const customCacheKey = `${path}:${xSite}`;
              console.log("existing in read", existing);

              return existing ? existing[customCacheKey] : undefined;
            },
            merge(existing = {}, incoming: any, { args, variables }: any) {
              const { path } = args;
              const xSite = variables?.xSite;
              if (!xSite) return existing;

              const customCacheKey = `${path}:${xSite}`;
              console.log("existing in merge", existing);

              return {
                ...existing,
                [customCacheKey]: incoming,
              };
            },
          },
        },
      },
  but even with this code the issue still persists, as I see following logs on server when navigating between these pages
    existing in read {
 '/inspiration/:da-dk': { __ref: 'page:1690' },
 '/inspiration/:sv-se': { __ref: 'page:1690' }
}

Can this be caused by the fact that even for different xSite the pages have the same id property in the response?

{
  "data": {
    "page": {
      "id": "1690",

Link to Reproduction

/

Reproduction Steps

No response

@apollo/client version

3.10.4

@IgorNovikov10
Copy link
Author

I also added custom

dataIdFromObject(responseObject: any) {
    switch (responseObject.__typename) {
      case "page":
        return `page:${responseObject.uniquePageId}`;
      default:
        return defaultDataIdFromObject(responseObject);
    }
  },

which now utilizes unique id instead of duplicating one:

now I get the following entries and references in read function:

  existing in read {
2024-07-23T16:45:31+03:00   '/ambassadorer/:da-dk': { __ref: 'page:da-dk-1551' },
2024-07-23T16:45:31+03:00   '/ambassadorer/:sv-se': { __ref: 'page:sv-se-1551' },
2024-07-23T16:45:31+03:00   '/inspiration/:sv-se': { __ref: 'page:sv-se-1690' },
2024-07-23T16:45:31+03:00   '/inspiration/:da-dk': { __ref: 'page:da-dk-1690' }
2024-07-23T16:45:31+03:00 }

but the issue is still in place :(

@alessbell
Copy link
Member

Hi @IgorNovikov10 👋 I have some suspicions about what is going on here, but in order to properly assess I'd need to see the whole application. Could you please share a runnable reproduction?

@alessbell alessbell added the 🏓 awaiting-contributor-response requires input from a contributor label Jul 23, 2024
@IgorNovikov10
Copy link
Author

@alessbell The problem can be reproduced only on a deployed instance by opening two pages with the same URL but on different domains (for example: https://dev-v2.focusnordic.se/ambassadorer and https://dev-v2.focusnordic.dk/ambassadorer) at the same time and trying to switch between pages in menu and returning back to the original page. This is the only way to see how the content of one site sometimes displays on another (it's a bit tricky to reproduce it locally by running next dev since there is only one domain localhost which can handle only one locale at the single period of the time)

Please let me know if you still need the source code or any other snippets of the code

@IgorNovikov10
Copy link
Author

IgorNovikov10 commented Jul 23, 2024

What I also noticed is that sometimes according to the logs on server I don't end up in the read function when opening/switching between navigation links fast and it looks like when I don't hit this function the content of the page resolved incorrectly and randomly shows either .se or .dk version on https://dev-v2.focusnordic.dk/ambassadorer

resolvedUrl /ambassadorer
xSite sv-se
existing in read {
  '/:sv-se': { __ref: 'page:sv-se-1164' },
  '/inspiration/:sv-se': { __ref: 'page:sv-se-1690' },
  '/om-oss/:sv-se': { __ref: 'page:sv-se-1660' },
  '/om-oss/foretagskultur/:sv-se': { __ref: 'page:sv-se-1661' },
  '/ambassadorer/:sv-se': { __ref: 'page:sv-se-1551' },
  '/ambassadorer/:da-dk': { __ref: 'page:da-dk-1551' },
  '/inspiration/:da-dk': { __ref: 'page:da-dk-1690' }
}
resolvedUrl /ambassadorer
xSite sv-se
resolvedUrl /ambassadorer
xSite sv-se
resolvedUrl /ambassadorer
xSite sv-se
resolvedUrl /ambassadorer
xSite sv-se
resolvedUrl /ambassadorer

@github-actions github-actions bot removed the 🏓 awaiting-contributor-response requires input from a contributor label Jul 24, 2024
@phryneas
Copy link
Member

Could it be that you are creating a long-living Apollo Client instance on the server?

We explicitly advise against that in the docs:

image

You risk mixing data from different users (or in your case, even sites) that should not be mixed. On the server, you should always create a new instance of ApolloClient for every incoming request.

@IgorNovikov10
Copy link
Author

@phryneas Hi, yep, currently I put ApolloProvider into pages/_app.tsx meaning it wraps the entire application

function MyApp({ Component, pageProps }: AppProps) {
  const apolloClient = initializeApollo(
    pageProps.reqData,
    pageProps.setHeaderFunction
  );
  
  ...

  return (
    <ApolloProvider client={apolloClient}>
      <Layout>
        <DataCacheProvider>
          <Component {...pageProps} />
        </DataCacheProvider>
      </Layout>
    </ApolloProvider>
  );
}

If I got you right, instead of this maybe worth trying to put it into [[...page]].tsx which is kind-of catch-all-routes component where I have getServerSideProps with apollo graphql queries

const Page: React.FC<PageProps> = ({
}) => {

  return (
    <AppContextProvider>
    ... perhaps AppoloProvider should be here?
    </AppContextProvider>
  );
};

export const getServerSideProps: GetServerSideProps = async (context) => {
  ... queries excuted here
};

export default Page;

@phryneas
Copy link
Member

phryneas commented Jul 24, 2024

No, the problem is that you probably have a file with

export apolloClient = new ApolloClient(....)

That means that if your server runs for a long time, this new ApolloClient happens once and will then be shared between every request against your server.

You need a setup that creates a new apolloClient instance for every incoming request.

Unfortunately, Vercel has already deleted all Page router examples on their end, but we still have one around that you can look up on how to do that correctly:

Here is the "core" of the setup:

https://github.com/apollographql/next-apollo-example/blob/main/pages/_app.js

And the important part is that here, in SSR the client instance is never saved to a global variable:

https://github.com/apollographql/next-apollo-example/blob/6de2adfcfeca3366eeb718d41118ff80ac706e83/lib/apolloClient.js#L55C1-L56C59

This page shows how to use it in getServerSideProps:
https://github.com/apollographql/next-apollo-example/blob/main/pages/index.js

@IgorNovikov10
Copy link
Author

@phryneas thanks! I can share my setup:

_app.tsx

import App, { AppProps } from "next/app";
import { ApolloProvider } from "@apollo/client";
import { initializeApollo } from "@/lib/apolloClient";

function MyApp({ Component, pageProps }: AppProps) {
  const router = useRouter();
  const apolloClient = initializeApollo(
    pageProps.reqData,
    pageProps.setHeaderFunction
  );

  return (
    <ApolloProvider client={apolloClient}>
      <Layout>
        <DataCacheProvider>
          <Component {...pageProps} />
        </DataCacheProvider>
      </Layout>
    </ApolloProvider>
  );
}

MyApp.getInitialProps = async (appContext: any) => {
  const appProps = await App.getInitialProps(appContext);
  const reqData = {...};

  return {
    ...appProps,
    pageProps: {
      ...appProps.pageProps,
      reqData,
      setHeaderFunction: appContext.ctx.res.setHeader,
    },
  };
};

export default MyApp;

@/lib/apolloClient

import {
  ApolloClient,
  InMemoryCache,
  ApolloLink,
  createHttpLink,
} from "@apollo/client";
import {
  accessTokenLink,
  authOperationsLink,
  persistBtcCartLink,
  refreshTokenLink,
  customerCookieLink,
} from "@/links";
import { config } from "./config";
import { getTokensFromCookies, getCustomerNumberFromCookie } from "@/utils";

let apolloClient: ApolloClient<any> | null = null;
let globalCache = new InMemoryCache({
  possibleTypes: config.apolloClient.cache.possibleTypes,
  typePolicies: config.apolloClient.cache.typePolicies,
  dataIdFromObject: config.apolloClient.dataIdFromObject,
});

const createApolloClient = (reqData: any, setHeaderFunction: any) => {
  const isSSR = typeof window === "undefined";

  const customerNumber = getCustomerNumberFromCookie(
    (key) => reqData.cookies[key],
    reqData.headers.host
  )();

  const httpLink = createHttpLink({
    uri: process.env.NEXT_PUBLIC_GRAPHQL_ENDPOINT,
    credentials: config.apolloClient.credentials,
    headers: {
      "x-site": reqData.headers["x-site"] ?? "sv-se",
      ...(customerNumber && { "x-customer": customerNumber }),
    },
  });

  const links = [
    accessTokenLink({
      getTokensFromCookies: getTokensFromCookies((key) => reqData.cookies[key]),
    }),
    authOperationsLink,
  ];

  links.push(httpLink);

  return new ApolloClient({
    ssrMode: isSSR,
    link: ApolloLink.from(links),
    cache: globalCache,
  });
};

export const initializeApollo = (reqData: any, setHeaderFunction: any) => {
  if (typeof window === "undefined") {
    return createApolloClient(reqData, setHeaderFunction);
  }

  if (!apolloClient) {
    apolloClient = createApolloClient(reqData, setHeaderFunction);
  }

  return apolloClient;
};

And getLayoutServerSideProps function from [[...page]].tsx

export const getLayoutServerSideProps = async (
  context: GetServerSidePropsContext,
  params: CommonLayoutSidePropsParams
): Promise<GetServerSidePropsResult<LayoutServerSideProps>> => {
  const { fetchPageData = false, dataFetchedServerSideOnly } = params;
  const { req, res, resolvedUrl } = context;

  const { cookie, ...headersWithoutCookie } = req.headers;

  const reqData = {
    headers: headersWithoutCookie,
    cookies: req.cookies,
  };

  const apolloClient: ApolloClient<NormalizedCacheObject> = initializeApollo(
    reqData,
    res.setHeader
  );
  const xSite = req.headers["x-site"] as string;

  // Define the Apollo queries
  const coreApolloQueries: Promise<ApolloQueryResult<any>>[] = [
    apolloClient.query({
      query: GetMenuDocument,
    }),
    apolloClient.query({
      query: GetMenuPrimaryDocument,
    }),
    apolloClient.query({
      query: GetTranslationsDocument,
    }),
  ];

 

  try {
    const [
      headerQueryResult,
      footerQueryResult,
      translationsQueryResult,
    ] = await Promise.all([
      ...coreApolloQueries,
    ]);

    return {
      props: {
        headerQueryResult,
        footerQueryResult,
        translationsQueryResult,
      },
    };
  } catch (error) {
    console.error("Error fetching data:", error);
    return {
      notFound: true,
    };
  }
};

@phryneas
Copy link
Member

cache: globalCache,

This is your problem. You seem to deliberately sharing a cache between requests.

The new InMemoryCache needs to be inside createApolloClient.

@alessbell alessbell added the 🏓 awaiting-contributor-response requires input from a contributor label Jul 24, 2024
@IgorNovikov10
Copy link
Author

IgorNovikov10 commented Jul 24, 2024

@phryneas Sure! First of all I want to cache the response for GetMenuDocument, GetMenuPrimaryDocument and GetTranslationsDocument (header and footer data basically, thate are static blocks) so as not to fetch this data every time the page changes (then the getServerSideProps function is launched and all queries executed again) and I also want to cache the page data based on page path (resolvedUrl)

apolloClient.query({
        query: GetPageDocument,
        variables: (() => {
          const [pathWithoutQuery, queryString] = resolvedUrl.split("?");
          const trailingSlashUrl = pathWithoutQuery.endsWith("/")
            ? pathWithoutQuery
            : `${pathWithoutQuery}/`;
          return {
            path: decodeURI(
              queryString
                ? `${trailingSlashUrl}?${queryString}`
                : trailingSlashUrl
            ),
            xSite,
            ...parseUrlQuery(queryString || ""),
          };
        })(),

which is also inside getServerSideProps so that when I go back to a page that has already been visited, I don't pull data again.

if I move the new InMemoryCache creation inside createApolloClient, then the requests are executed every time regardless of the fetchPolicy set on the query.

Byt at the same time I want the cache not to persist between different sites, that's why I tried to create unique cache keys based on xSite variable that I pass to query and uniquePageId for the page reference that is different for the same routes on different locales like http://site.com/test-page and http://site.se/test-page

@github-actions github-actions bot removed the 🏓 awaiting-contributor-response requires input from a contributor label Jul 25, 2024
@phryneas
Copy link
Member

if I move the new InMemoryCache creation inside createApolloClient, then the requests are executed every time regardless of the fetchPolicy set on the query.

Yes. This is our recommendation. During SSR, every time you render, you should make all requests again. Everything else leaks private data between different visitors of your site, can cause spikes in memory consumption, will server potentially outdated data etc. etc.

Apollo Client is made to exist for a long time in a browser window, not on a server. If you want to deduplicate requests on a server, you can use mechanisms provided by your SSR framework. I know that Next.js provides a fetch cache in the App router, unfortunately I cannot tell you if they provide a fetch cache for the Pages router.

Byt at the same time I want the cache not to persist between different sites, that's why I tried to create unique cache keys based on xSite variable that I pass to query and uniquePageId for the page reference that is different for the same routes on different locales like http://site.com/test-page and http://site.se/test-page

If you really want to go that route (again, against our recommendation and on your own risk!), I would suggest that you create one cache instance per site instead of cramming data for all sites into one cache. InMemoryCache is not designed for that level of separation.

@IgorNovikov10
Copy link
Author

@phryneas Maybe there is a small misunderstanding here

When refreshing the page (during the first render) I of course perform queries to get data, I meant that when I navigate between the pages in the browse using next/link, I want to see the cached result when I return to the page I was already on. For this I use fetchPolicy cache-first. But when I moved the initialization of new InMemoryCache inside createApolloClient function

import {
  ApolloClient,
  InMemoryCache,
  ApolloLink,
  createHttpLink,
} from "@apollo/client";

let apolloClient: ApolloClient<any> | null = null;

const createApolloClient = (reqData: any, setHeaderFunction: any) => {
  const isSSR = typeof window === "undefined";

 ...

  let cache = new InMemoryCache({
    possibleTypes: config.apolloClient.cache.possibleTypes,
    typePolicies: config.apolloClient.cache.typePolicies,
    dataIdFromObject: config.apolloClient.dataIdFromObject,
  });

  return new ApolloClient({
    ssrMode: isSSR,
    link: ApolloLink.from(links),
    cache: cache,
  });
};

export const initializeApollo = (reqData: any, setHeaderFunction: any) => {
  if (typeof window === "undefined") {
    return createApolloClient(reqData, setHeaderFunction);
  }

  if (!apolloClient) {
    apolloClient = createApolloClient(reqData, setHeaderFunction);
  }

  return apolloClient;
};

this cache inside the browser does not work, since once I navigated to a new page using next/link the getLayoutServerSideProps function is called again which re-initializes apolloClient

export const getServerSideProps: GetServerSideProps = async (context) => {
...
 const apolloClient: ApolloClient<NormalizedCacheObject> = initializeApollo(
reqData,
res.setHeader
); 

...
};

export default Page;

and accordingly creates a new cache instance. How can this be fixed so that the cache only works during the client session?

@phryneas
Copy link
Member

Moving this into createApolloClient will keep the same client and cache during the full client session, not throwing anything away or create a new one, but will create a new one on the server every time.

Keep in mind that there is no such thing as a "server session" in Next.js.
Every SSR request is completely independent of a potential long-running user session in the browser.

You can have a long session in the Client, but never on the browser. It's always a new request from an unknown anonymous user.

@IgorNovikov10
Copy link
Author

@phryneas yes, I think the problem is that Next js triggers the getServerSideProps function execution every time I navigate between router links in the browser (during the client session), which in turn recreates the Apollo/cache instance on each request and because of this I have no way to cache the data

@IgorNovikov10
Copy link
Author

@phryneas full client session in your interpretation, this is the period of time the user is on the current page, which ends when following a new next router link in the SPA application?

@phryneas
Copy link
Member

I think the problem is that Next js triggers the getServerSideProps function execution every time I navigate between router links in the browser (during the client session), which in turn recreates the Apollo/cache instance on each request and because of this I have no way to cache the data

Yes, that's how the SSR rendering in the Next.js pages router works.

in your interpretation, this is the period of time the user is on the current page, which ends when following a new next router link in the SPA application?

No, from the moment a user opens your page in the browser until they navigate to another website or close the tab. You might see these user-side requests, but the ApolloClient instance in the browser should keep existing independently of that in your setup. You have one long-running session on the browser, enhanced over time by many short-lived sessions on the server.

@IgorNovikov10
Copy link
Author

IgorNovikov10 commented Jul 25, 2024

@phryneas Thanks. Do you have an idea on how to persist Apollo cache between page visits on client-side in NextJS and not to load data for the pages that have been visited before? I tried to test this example https://github.com/apollographql/next-apollo-example and it looks like during the long-running session on the browser, when navigating between pages, it still performs Apollo queries in getServerSideProps, createApolloClient and new InMemoryCache get called every time

@phryneas
Copy link
Member

phryneas commented Jul 25, 2024

it still performs Apollo queries in getServerSideProps, createApolloClient and new InMemoryCache get called every time

That is the Next.js behaviour when you use getServerSideProps, it has nothing to do with Apollo Client.
It will try to server-render pages on every navigation, and if that page needs api data it will query that on the server, too.

This is a feature of your framework. Next.js is geared towards SSR.

@IgorNovikov10
Copy link
Author

@phryneas Thanks a lot for your help and opinion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants