-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Context.overwrite truthy so existing array always voided and so undefined when arg in merge method of apollo cache #9543
Comments
I suspect The goal of this overwriting after refetch is to replace existing data with refetched data without combining them together, since the refetched data is typically more fresh/complete/recent than the existing data. However, it's still important to call With that explanation of the default behavior out of the way, some (hopefully) good news: this behavior is configurable! You can pass an option called If you want to make the pre-v3.4 behavior ( new ApolloClient({
defaultOptions: {
watchQuery: {
// We recommend "overwrite" instead of "merge", but here's how
// you can restore the "merge" behavior by default for all queries.
refetchWritePolicy: "merge",
},
},
}) Again, I'm not 100% sure about my initial diagnosis, so please give |
@benjamn you absolute diamond! const query_options = useMemo(() => ({
onCompleted: onResultCallback, fetchPolicy: fetch_policy, refetchWritePolicy: "merge", notifyOnNetworkStatusChange: true, returnPartialData: false, errorPolicy: "all"
}), [fetch_policy, onResultCallback]);
const [sendRequest, { error }] = useLazyQuery(gql`${query}`, { ...query_options, variables });
// ...
// later on in execution we call sendRequest with updated request variables like so...
sendRequest({ variables: variables_ref.current }); The reason merge is important for our use case is that there are 3 types of response expected from any request with a matching UUID, the first is usually a null response that indicates the data is not yet available so polling continues, the second is a full result and the third is a delta update. We separate these out using isDelta as a keyField (see config above) as it's this result property that indicates the 3 possible result types, null, false, and true respectively. I want to add code to our merge function to only merge in non-null results. Which I should now be able to do. Thank you so much! |
@benjamn so as you can see in the snippet posted in my previous reply, we're not retrieving the refetch method from the lazy query hook, we use the send method, is that equivalent? Should overwrite still be true by default in this scenario? |
Oh, thanks for reminding me: we currently have a bug that the That bug is a top priority for me before we publish the first release candidate of v3.6, so I've added this issue to the Release 3.6 milestone to make sure I comment here once that's fixed in prerelease. |
@atomless The problem with the Specifically, after updating to the latest v3.6 beta version, you should no longer see |
We've merged |
Hi All.
This feels like a bug in apollo client or in the inMemoryCache but could of course be something I'm doing wrong. Basically, using the chrome dev tools extension for apollo I can see that for the uuid of the outgoing/incoming request there are already multiple cached results but by the time code execution reaches the merge method invoked by Apollo Cache, the "existing" arg is always undefined.
Intended outcome:
I'm using Apollo Cache and want to write custom code to conditionally merge incoming results with existing cached results.
Actual outcome:
The existing results arg in the merge method invoked by Apollo Cache is always undefined, even though I can see matching entries in the cache using the apollo extension in chrome dev tools. Also if I break in the code and step back up the execution stack I can see that existing did have entries but was voided as "context.overwite" is truthy without me setting it so.
How to reproduce the issue:
When I set a breakpoint in the merge function and then step back in the execution stack I can see that "existing" DOES have entries but it is voided when execution reaches the apollo client file policies.ts on lines 862 to 664, due to the context.overwrite boolean being truthy:
After execution of the lines above, when console logging "existing" in the merge function below, it's value is ALWAYS undefined. I don't set the overwrite option anywhere and it's default is supposed to be false. What am I missing, or is this a bug?
Here's my basic config and the merge method in question.
Versions
"@apollo/client": "3.5.10",
"graphql": "16.0.1",
"react": "17.0.2",
System:
OS: macOS 10.15.7
Binaries:
Node: 16.13.0 - ~/.nvm/versions/node/v16.13.0/bin/node
npm: 8.1.0 - ~/.nvm/versions/node/v16.13.0/bin/npm
Browser:
Chrome: Version 99.0.4844.74 (Official Build) (x86_64)
The text was updated successfully, but these errors were encountered: