fix: Allow fetchMore() to be used with 'no-cache' fetchPolicy

[mogelbrod]

Happy to add test(s) if someone could verify that I've provided a valid fix for the issue.

This PR is basically a continuation of #5239 (comment)

Fixes #5239

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests

[apollo-cla]

@mogelbrod: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[mogelbrod]

WIthout this markQueryResult appears to write to the cache:

apollo-client/src/core/QueryManager.ts

Lines 505 to 519 in e9be4be

	private markQueryResult(
	queryId: string,
	result: ExecutionResult,
	{
	fetchPolicy,
	variables,
	errorPolicy,
	}: WatchQueryOptions,
	fetchMoreForQueryId?: string,
	) {
	if (fetchPolicy === 'no-cache') {
	this.setQuery(queryId, () => ({
	newData: { result: result.data, complete: true },
	}));
	} else {

[mogelbrod]

This argument might be better off as an object?

[mogelbrod]

Required for access from ObservableQuery.updateQuery

[codecov]

Codecov Report

Merging #6023 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6023      +/-   ##
==========================================
- Coverage   95.20%   95.16%   -0.05%     
==========================================
  Files          88       88              
  Lines        3674     3679       +5     
  Branches      873      906      +33     
==========================================
+ Hits         3498     3501       +3     
- Misses        154      155       +1     
- Partials       22       23       +1     

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9be4be...29acd01. Read the comment docs.

[mikedizon]

Waiting for this!

[mogelbrod]

Would love to get this into the v3.0 release, just waiting for someone knowledgeable (@hwillson perhaps?) of the code base to give some feedback on the approach I used before writing the tests for it.

[mirague]

Would love to hear from the team on this!

[korengrip]

@mogelbrod any news?
Really need this feature.

[mogelbrod]

@mogelbrod any news?
Really need this feature.

We haven't started our migration to Apollo yet, so I haven't been able to check if this merged v3 PR solved this issue. Hopefully it did though.

[korengrip]

@mogelbrod any news?
Really need this feature.

We haven't started our migration to Apollo yet, so I haven't been able to check if this merged v3 PR solved this issue. Hopefully it did though.

It seems like it didn't solve the issue.

[hwillson]

Thanks for this @mogelbrod and sorry for the delay. This work was superseded by the work in #6221, which set the fetchMore fetchPolicy default to no-cache (see the description in that PR for the reasoning):

apollo-client/src/core/ObservableQuery.ts

Lines 413 to 418 in 7041fe7

	// The fetchMore request goes immediately to the network and does
	// not automatically write its result to the cache (hence no-cache
	// instead of network-only), because we allow the caller of
	// fetchMore to provide an updateQuery callback that determines how
	// the data gets written to the cache.
	fetchPolicy: "no-cache",

Thanks!