Singleton ApolloClient instance approach, potential issues?

[ViktorShev]

Could this approach present any issues? In a previous project we had kind of the same set up with Next's page router, where we shared the same Apollo instance throughout the application without any issues.

Currently we are using NextJS 14 with the app router

'use client'

import { NormalizedCacheObject } from '@apollo/client'
import {
  ApolloClient,
  ApolloNextAppProvider,
  InMemoryCache
} from '@apollo/experimental-nextjs-app-support'
import { PropsWithChildren, ReactElement } from 'react'

import authLink from '~/service_providers/graphql/configuration/auth_link'
import splitLink from '~/service_providers/graphql/configuration/link'

const link = authLink.concat(splitLink)
const client = new ApolloClient<NormalizedCacheObject>({
  cache: new InMemoryCache(),
  link
})

export function makeClient (): ApolloClient<NormalizedCacheObject> {
  return client
}

export function ApolloWrapper ({ children }: PropsWithChildren): ReactElement {
  return (
    <ApolloNextAppProvider makeClient={makeClient}>
      {children}
    </ApolloNextAppProvider>
  )
}

My use case is that I need to use the same ApolloClient instance outside the context of React, more specifically, I have an FirebaseAuthenticationProvider class that has a method to retrieve the current user's database ID. The user token in sent automatically through auth link.

...
    try {
      if (!isNil(idToken) && isNil(userId)) {
        const { data } = await makeClient().query({
          query: currentUser
        })
        userId = data?.currentUser?.id
      }
    } catch (error: any) {
      return err(new UnexpectedError('Could not retrieve the id associated with the user\'s token'))
    }

    return ok(userId)

This class is retrieved through a function that will only instantiate the class IF there was a change in authentication state (e.g a user logged in), this ensures that the class will not "operate" on methods without a user, if the class is available, it means there is a user.

let authenticationProvider: AuthenticationProvider | null = null

export function setAuthenticationProvider (provider: AuthenticationProvider): void {
  authenticationProvider = provider
}

export async function getAuthenticationProvider (): AsyncResult<AuthenticationProvider, MissingDependencyError> {
  if (authenticationProvider === null) {
    await initializeFirebase()
    await tokenInitializedPromise
    setAuthenticationProvider(new FirebaseAuthenticationProvider())
  }

  return ok(authenticationProvider)
}

And is used on my authentication link like so:

const authLink = setContext(async (_, { headers }) => {
  const authProvider = await getAuthenticationProvider()

  let token

  if (authProvider.isOk()) {
    const tokenResult = await authProvider.value.getToken()
    if (tokenResult.isOk()) {
      token = tokenResult.value
    }
  }

  return {
    headers: {
      ...headers,
      authorization: !isNil(token) ? `Bearer ${token}` : ''
    },
    credentials: 'include'
  }
})

export default authLink

the method authProvider.getToken() does not have a backend request, that's why I'm able to call getToken without falling into a circular dependency of needing Apollo already set up in the auth provider for backend requests and the auth provider being needed for setting up Apollo.

The reason for needing the same instance throughout my application is so that once the authentication provider fetched a user from the backend, it can cache it and not hit the server every time I need to retrieve some user information again due to makeClient returning a different instance with a new cache.

Note: I imperatively clear the cache on user log out with .clearStore()

I assume this won't have problems with SSR either since the InMemoryCache from the experimental package acts a "enqueuer" of cache writes during SSR, and once we are in the client context, the cache is restored by executing those writes, unless I understood the RFC incorrectly: https://github.com/apollographql/apollo-client-nextjs/blob/pr/RFC-2/RFC.md#library-design-regarding-ssr

Also looking at the source code the ApolloNextAppProvider reuses the same client instance as a singleton too.

So far I haven't noticed any problems besides not being able to call makeClient() in server components like a layout, Next throws a webpack error:

app/profile/layout.tsx (8:25) @ makeClient
 ⨯ TypeError: (0 , _main_app_components_ApolloWrapper_ApolloWrapper__WEBPACK_IMPORTED_MODULE_2__.makeClient) is not a function
    at ProfileLayout (./app/profile/layout.tsx:16:109)
    at stringify (<anonymous>)
    at AsyncLocalStorage.run (node:async_hooks:338:14)
digest: "1887363878"

But just wanted confirmation if my assumption is correct and if there could be any other potential issues with this pattern (and potentially a hint towards fixing the webpack error 😬)

Thanks!

[jerelmiller]

Hey @ViktorShev 👋

We don't recommend you share the same instance between requests otherwise you risk leaking user data. If you're authenticating once outside of a request, only a single user could login to your app at a time (or at least one user per running server instance). Even if you managed to solve that issue, using a single instance of the client means that you would be merging multiple user's data into the same InMemoryCache instance and that could leak user data between users.

That makeClient prop exists for this reason. This package will create a new client per-request to prevent this issue. The client is the same instance however during the duration of the request. If you kick off multiple queries in the same request for example, you'll be using the same client/cache instance for each of those queries.

If you're trying to avoid making that authentication call to the remote service on every request, I'd recommend caching and storing that value outside Apollo Client with something like Redis or your own key/value store that can distinguish between users.

[ViktorShev]

@jerelmiller Hey! thanks for the quick response.

I don't seem to be experiencing any of the issues you mention, I am still able to log in into my application with different users and interact with it in parallel, checking the Apollo cache of each browser instance they also don't seem to be leaking into each other, be it after running mutations or querying data:

Maybe I missed some details in my initial issue, would you like me to provide some more clarity on anything?

I understand this might not be the best approach and will be looking into something else, but now I'm intrigued as to why this seems to be working.

[jerelmiller]

Ah I missed the idToken part of your initial description and went straight to the singleton instance of the authorization mechanism. So are probably fine here as long as your clients send those tokens to distinguish them. Ignore the part of my last message about only allowing a single user to login.

As for your screenshot, you're not going to see issues on records that have different cache IDs, like the user records themselves. Where you start to get issues with data leaking is when you're querying for records that have user-specific data attached to a shared entity.

As an example, let's say you have a schema like this:

type Query {
  post(id: ID!): Post
}

type Post {
  id: ID!
  canEdit: Boolean!
  contents: String!
}

where canEdit is dependent on whether or not the logged-in user authored the Post.

And let's say that when users visit a particular post, it issues the following query:

query($postId: ID!) {
  post(id: $postId) {
    id
    canEdit
    contents
  }
}

Now let's say the author of post 1 is the first to visit that post. It will be cached as such:

{
  ROOT_QUERY: {
    "post({\"id\":1})": { __ref: "Post:1" }
  },
  "Post:1": {
    id: 1,
    canEdit: true,
    contents: "Lorem ipsum",
  }
}

Now lets say another user logs in and visits the same post, which issues the same query. Because we can fulfill the data from the cache, this means that we return the cached data for that post. Oops, now we've just given that user the ability to edit that post.

Its these types of situations where having a shared cache for all users can be dangerous. Its very common that at least some of your records are visible among a subset of your users, and also common that your schema embeds user-specific information with those in some form. You can imagine how bad it might be if those records contain PII.

Does this help illustrate the situation?

[ViktorShev]

Okay that makes a lot of sense now!

I will be closing the issue now as it is very evident that this approach can cause some big problems.

Thanks for your time!

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

[jerelmiller]

You're welcome! Glad that helps 🙂

[phryneas]

I would also recommend that you look at this comment for an alternative pattern - you would set the auth token on the client after the client was created by makeClient. Your FirebaseAuthenticationProvider could be instantiated by a child of your ApolloProvider to have access to the current client.

#103 (comment)