useQuery returns results with different references when custom `read` function defined on field policy

[a-churchill]

Issue Description

Overview

If you define a read function on the field policy for a given field which returns a different value than the current one, then the data returned by the first instance of useQuery will not have the same reference as the data returned by all subsequent instances, even though the data must be equivalent.

This is not just an academic point - if you have a cache of size 1 that depends on referential equality for the same query results, it will be busted by this behavior. That's what happened in my app actually - this bug causes a full crash in production 😅

Results of my investigation

Note I know the rules say not to speculate about the cause - I am including this because I think (hope!) it's more useful than pure speculation. But feel free to ignore it if you know the issue lies elsewhere!

I've spent quite some time looking into why this happens since I feared it was a configuration issue on my end, and so I can provide some hopefully useful pointers based on that investigation.

Each call to useQuery creates and uses its own ObservableQuery instance, both to subscribe to updates to a query's results, and to get the current value for that query. When useQuery is called with the default fetch policy ("cache-first"), that will trigger a network request.

This network request ultimately happens in QueryManager.getObservableFromLink: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/core/QueryManager.ts#L1089 This handles deduplication across multiple instances of useQuery with the same variables, subscribing all of them to the same observable. Once the network returns a result, QueryInfo.markResult will be called for each ObservableQuery (<=> useQuery) instance: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/core/QueryManager.ts#L1131 Note that this result comes straight from the network, and does not have any custom read functions applied yet.

markResult essentially does 2 things:

1. Write the query results to the cache: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/core/QueryInfo.ts#L406-L411
2. Call cache.diff to read the results back from the cache: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/core/QueryInfo.ts#L463

Notably, the read functions don't affect step 1 at all - they aren't even called. They are only called for step 2.

Now the same happens for every subsequent instance of useQuery. One key point, though: markResult mutates its input here: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/core/QueryInfo.ts#L481 This means that all subsequent listeners to the fetch link result will actually see the result of cache.diff, as opposed to the result from the network.

Ok, so where does the problem happen in all of this? I believe the key is these lines: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/cache/inmemory/entityStore.ts#L133-L135 and these at the end of the if block: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/cache/inmemory/entityStore.ts#L185-L186

That EntityStore.merge method is called during the write phase of markResult. The first time it writes the data, it's writing the raw network result (without read functions applied). But the second time it writes the data, because of the argument mutation I pointed out above, it will actually be writing the result of cache.diff, with the read functions applied. This does not equal the existing entry in this.data, so that field will be dirtied (and, therefore, all its parents will be dirtied too). This means the next time cache.diff runs, it will recompute the result for this query (since it's been dirtied) and return a different reference.

This also explains why the second, third, fourth, etc. instance all return the same reference: they're all calling writeQuery and cache.diff with the result of the first call's cache.diff, so they all hit the cache.

So to summarize, it seems like there is a potential one line fix - getting rid of this argument mutation: https://github.com/apollographql/apollo-client/blob/6875b6239d088017c392f93105b18b58f4def4b2/src/core/QueryInfo.ts#L481 Of course I'm sure that line has other reasons for existing so I doubt it will be that easy! But that's at least hopefully a place to start.

Workarounds

Turning on canonizeResults is sufficient to work around this bug (as long as the read function returns the same reference in subsequent calls), but this has other performance penalties and is not the default option which is why I'm still submitting this as a bug and I think it's important.

Link to Reproduction

https://codesandbox.io/s/apollo-client-bug-double-references-kfckyg?file=/src/index.jsx

Reproduction Steps

N/A - the CodeSandbox shows a reproduction without any interaction :)

[jerelmiller]

@a-churchill thanks so much for the detailed deep dive! This is certainly very helpful!

@benjamn this looks very similar to that bug we were looking at a while ago together and if I remember correctly, the line of code mentioned in the description above ^ is the same line we also attributed to that bug. Does this ring a bell for you?

[a-churchill]

Whoops, accidentally closed because I merged a PR in a private unrelated repo with the string "fix https://github.com/apollographql/apollo-client/issues/11002" in the description and GitHub decided that meant this issue should be closed 😆