Server-into-client cache merging in with-apollo example is incorrect

[rtrembecky]

Verify canary release

• I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.6.0: Fri Sep 15 13:41:28 PDT 2023; root:xnu-8796.141.3.700.8~1/RELEASE_ARM64_T6000
    Binaries:
      Node: 16.20.2
      npm: 8.19.4
      Yarn: 1.22.19
      pnpm: N/A
    Relevant packages:
      next: 13.4.1
      eslint-config-next: 13.4.1
      react: 18.2.0
      react-dom: 18.2.0

Which example does this report relate to?

with-apollo, with-apollo-and-redux

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

In the with-apollo example, the code in initializeApollo function is a very incorrect default behavior for merging the server-side cache into the client cache. Let me summarize the arch first:

• in getServerSideProps in ssr.js page:
  • initializeApollo() without initialState is used to create an apolloClient
  • apolloClient.query(...) is called
  • addApolloState(...) is called to pass Apollo's cache into pageProps
• in _app.js component:
  • useApollo(pageProps) is used to get apolloClient with server cache integrated (hydrated)
• in useApollo:
  • cache state is taken from pageProps
  • initializeApollo(state) is called to get apolloClient with server cache integrated (hydrated)

Note that getServerSideProps runs also on client-side navigation, not only for the first SSR render of the app.

The behavior of initiallizeApollo with initialState is what concerns me and what brought us nasty bugs on production.
I believe this function should mimic the internal Apollo's mechanism for updating the cache when new data for a query are fetched.

Finally, explanation of behavior differences with bug examples:

• array field merging:
  • Apollo - arrays are NOT merged (incoming array replaces the existing) unless specified otherwise in the cache definition
    • holds for both arrays of normalized and non-normalized objects
    • docs: https://www.apollographql.com/docs/react/caching/cache-field-behavior#merging-arrays
  • initializeApollo - arrays ARE merged as a "set" - incoming array is concatenated with existing array elements that are not present in the incoming array
    • holds for both arrays of normalized and non-normalized objects
    • ⚠️ bug - objects may contain updated properties in a new query call, e.g. timestamps. The normalized object will only have its cache reference in the array, which is fine, but a non-normalized object will be duplicated in the array as it won't pass the isEqual check with the incoming element. (This is what we encountered in prod on client navigation.)
• non-normalized object field merging:
  • Apollo - the objects are NOT merged (the new object replaces the existing) unless specified otherwise in the cache definition
    • docs: https://www.apollographql.com/docs/react/caching/cache-field-behavior#merging-arrays-of-non-normalized-objects
    • docs for normalized: https://www.apollographql.com/docs/react/caching/overview/#4-store-normalized-objects
  • initializeApollo - deepmerge lib is used, it deep MERGES the whole cache object
    • ⚠️ bug - I'm yet to find a good example, it would be probably something like "I want the new query call to REMOVE some property of my non-normalized object but initializeApollo KEEPS it", which is arguably a not very common use case

Looking at the with-apollo history, I found the duplication was meant to be fixed in 2020 (issue #19759, PR #19812), though wrongly.

Expected Behavior

Cache merge should be in line with Apollo's internal cache update mechanism. It should merge only normalized objects and fields (objects/arrays) based on the user's cache definition.

To Reproduce

• use the with-apollo example
• create an Apollo query returning array of non-normalized objects that changes with each call
  • it's enough for one object to change one property, e.g. timestamp
  • own apollo server is probably needed for this
• OPTION 1:
  • use the query in getServerSideProps (apolloClient.query()) and in Page component (useQuery()) for two different pages
  • log or visualize data from useQuery on page
  • create a <Link> to the second page
  • visit page 1 via the address bar - the client cache gets updated (hydrated) with server data for the first time, no other client network request is done
  • visit page 2 via the <Link> - the client cache gets updated with server data coming in pageProps, no other client network request is done
    • ⚠️ duplicated array element should be logged/visible
• OPTION 2:
  • use the query in getServerSideProps (apolloClient.query()) and in Page component (useQuery()) for a single page
  • log or visualize data from useQuery on page
  • create a second page
  • create backlinks between both pages via <Link>
  • visit page 1 via the address bar - the client cache gets updated (hydrated) with server data for the first time, no other client network request is done
  • visit page 2 via the <Link>
  • visit page 1 via the <Link> - the client cache gets updated with server data coming in pageProps, no other client network request is done
    • ⚠️ duplicated array element should be logged/visible

[rtrembecky]

Furthermore, the description above only slightly touches that user can have their own InMemoryCache definition (own keyFields, merge property with custom merge function, etc.). The proper solution should account for the cache definition the exact same way as Apollo does.
Quick link to Apollo files I plan to check: https://github.com/apollographql/apollo-client/blob/main/src/cache/inmemory/entityStore.ts

[rtrembecky]

Post on Apollo's Discord: https://discord.com/channels/1022972389463687228/1186628052193652736/1186628052193652736

[nextjs-bot]

This issue has been automatically marked as stale due to two years of inactivity. It will be closed in 7 days unless there’s further input. If you believe this issue is still relevant, please leave a comment or provide updated details. Thank you.

[rtrembecky]

Well, I see the with-apollo for Pages has been replaced by App router example, soo, sure - the wrong guide for Pages is now no guide for Pages, which is probably fine 😄

[github-actions]

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.