Maximum call stack size exceeded possible in query with object queried at multiple graphql paths

[abergenw]

I have a query somewhat with the following structure:

query TestQuery {
  authenticatedUser {
    ...UserFragment
      
    company {
      users {
        ...UserFragment
      }
    }
  }
}

fragment UserFragment on User {
  firstName
  company {
    id
  }
}

If the same company (same id) is returned both under authenticatedUser > company and authenticatedUser > company > users > company, it seems like the same object instance is returned from apollo-cache-inmemory. This has all sorts of implications, not least as it can lead to the resulting object having a cyclic nature making Apollo cause a RangeError when using isEqual.

I suspect this is because in the above example the company selection set is cached (with users) and thus not evaluated correctly for authenticatedUser > company > users > company (without users). Maybe @benjamn can confirm this?

Versions

System:
OS: macOS High Sierra 10.13.6
Binaries:
Node: 8.11.4 - /usr/local/bin/node
Yarn: 1.10.1 - /usr/local/bin/yarn
npm: 5.6.0 - /usr/local/bin/npm
Browsers:
Chrome: 70.0.3538.77
Safari: 12.0
npmPackages:
apollo-cache-inmemory: 1.3.7 => 1.3.7
apollo-client: 2.4.5 => 2.4.5
apollo-link-batch-http: 1.2.3 => 1.2.3
react-apollo: 2.2.4 => 2.2.4
npmGlobalPackages:
apollo-codegen: 0.10.5

[benjamn]

Can you put together a CodeSandbox reproduction? We shouldn't be returning the same company object for authenticatedUser > company as we're returning for the ... users > company objects, since the selection sets have a different structure. However, the same company object could appear multiple times for different users. That by itself shouldn't create cycles, but maybe there's something else happening.

[abergenw]

Here's a simple reproduction:

https://codesandbox.io/s/94w34wx1ow

I also quickly tried creating a test only for apollo-cache-inmemory but for some reason I couldn't get the problem to occur while only testing readQueryFromStore.

[benjamn]

@abergenw I believe I've tracked this bug down to the merge function in readFromStore.ts, and I've submitted a PR to be extra careful never to modify cached result objects. I'm still very much planning to add a regression test, though that's been a bit tricky (as you noticed above). In the meantime, I've published a beta version to npm. Please try updating to apollo-cache-inmemory@1.3.9-beta.1 when you have the chance!

[abergenw]

I can confirm that apollo-cache-inmemory@1.3.9-beta.1 works like a charm. Good work @benjamn!

[benjamn]

Since apollo-cache-inmemory@1.3.9 has been published, I think this is fixed. Thanks for reporting this issue, @abergenw!