Fall back to cache-first after cache-and-network or network-only. #6353
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When someone chooses cache-and-network or network-only as their initial FetchPolicy, they almost certainly do not want future cache updates to trigger unconditional network requests, which is what repeatedly applying the cache-and-network or network-only policies would seem to require. Instead, when the cache reports an update after the initial network request, subsequent network requests should be triggered only if the cache result is incomplete. This behavior corresponds exactly to switching to a cache-first FetchPolicy, which we can achieve by modifying options.fetchPolicy in QueryManager#fetchQueryObservable for the next fetchQueryObservable call, using the same options object that the Reobserver always passes to fetchQueryObservable. Note: if these FetchPolicy transitions get much more complicated, we might consider using some sort of state machine to capture the transition rules. Should fix #6305, and a few other related issues (TBD).
benjamn
added a commit
that referenced
this pull request
Jun 9, 2020
This fixes an issue found by @darkbasic while working with optimistic updates: #6183 (comment) The cache-first FetchPolicy is important not just because it's the default policy, but also because both cache-and-network and network-only turn into cache-first after the first network request (#6353). Once the cache-first policy is in effect for a query, any changes to the cache that cause the query to begin reading incomplete data will generally trigger a network request. However, if the source of the changes is an optimistic update for a mutation, it seems reasonable to avoid the network request during the mutation, since there's a good chance the incompleteness of the optimistic data is only temporary, and the client might read a complete result after the optimistic update is rolled back, removing the need to do a network request. Of course, if the non-optimistic read following the rollback is incomplete, a network request will be triggered, so skipping the network request during optimistic updates does not mean ignoring legitimate incompleteness forever. Note: we already avoid sending network requests for queries that are currently in flight, but in this case it's the mutation that's in flight, so this commit introduces a way to prevent other affected queries (which are not currently in flight, themselves) from hitting the network.
benjamn
added a commit
that referenced
this pull request
Jun 10, 2020
This fixes an issue found by @darkbasic while working with optimistic updates: #6183 (comment) The cache-first FetchPolicy is important not just because it's the default policy, but also because both cache-and-network and network-only turn into cache-first after the first network request (#6353). Once the cache-first policy is in effect for a query, any changes to the cache that cause the query to begin reading incomplete data will generally trigger a network request, thanks to (#6221). However, if the source of the changes is an optimistic update for a mutation, it seems reasonable to avoid the network request during the mutation, since there's a good chance the incompleteness of the optimistic data is only temporary, and the client might read a complete result after the optimistic update is rolled back, removing the need to do a network request. Of course, if the non-optimistic read following the rollback is incomplete, a network request will be triggered, so skipping the network request during optimistic updates does not mean ignoring legitimate incompleteness forever. Note: we already avoid sending network requests for queries that are currently in flight, but in this case it's the mutation that's in flight, so this commit introduces a way to prevent other affected queries (which are not currently in flight, themselves) from hitting the network.
benjamn
added a commit
that referenced
this pull request
Jul 27, 2020
PR #6353 explains the rationale for switching to a cache-first FetchPolicy after an initial cache-and-network or network-only policy. When #6365 was implemented, options.fetchPolicy was examined only once, at the beginning of fetchQueryObservable, so the timing of changing options.fetchPolicy did not matter as much. However, fixing #6659 involves checking the current options.fetchPolicy whenever the QueryData class calls this.currentObservable.getCurrentResult(), so it's now more important to delay changing options.fetchPolicy until after the first network request has completed.
benjamn
added a commit
that referenced
this pull request
Jul 27, 2020
PR #6353 seemed like a clever zero-configuration way to improve the default behavior of the cache-and-network and network-only fetch policies. Even though the word "network" is right there in the policy strings, it can be surprising (see #6305) to see network requests happening when you didn't expect them. However, the wisdom of #6353 was contingent on this new behavior of falling back to cache-first not creating problems for anyone, and unfortunately that hope appears to have been overly optimistic/naive: #6677 (comment) To keep everyone happy, I think we need to revert #6353 while providing an easy way to achieve the same effect, when that's what you want. The new options.nextFetchPolicy option allows updating options.fetchPolicy after the intial network request, without calling observableQuery.setOptions. This could be considered a breaking change, but it's also a regression from Apollo Client 2.x that needs fixing. We are confident options.nextFetchPolicy will restore the #6353 functionality where desired, and we are much more comfortable requiring the explicit use of options.nextFetchPolicy in the future.
This was referenced Jul 27, 2020
benjamn
added a commit
that referenced
this pull request
Jul 27, 2020
PR #6353 seemed like a clever zero-configuration way to improve the default behavior of the cache-and-network and network-only fetch policies. Even though the word "network" is right there in the policy strings, it can be surprising (see #6305) to see network requests happening when you didn't expect them. However, the wisdom of #6353 was contingent on this new behavior of falling back to cache-first not creating problems for anyone, and unfortunately that hope appears to have been overly optimistic/naive: #6677 (comment) To keep everyone happy, I think we need to revert #6353 while providing an easy way to achieve the same effect, when that's what you want. The new options.nextFetchPolicy option allows updating options.fetchPolicy after the initial network request, without having to call observableQuery.setOptions. Specifically, passing { nextFetchPolicy: "cache-first" } for network-only or cache-and-network queries should restore the behavior of #6353. This could be considered a breaking change, but it's also a regression from Apollo Client 2.x that needs fixing. We are confident options.nextFetchPolicy will enable the #6353 functionality where desired, and we are much more comfortable requiring the explicit use of options.nextFetchPolicy going forward.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When someone chooses
cache-and-networkornetwork-onlyas their initialFetchPolicy, they almost certainly do not want future cache updates to trigger unconditional network requests, which is what repeatedly applying thecache-and-networkornetwork-onlypolicies would seem to require.Instead, when the cache reports an update after the initial network request, subsequent network requests should be triggered only if the cache result is incomplete.
This behavior corresponds exactly to switching to a
FetchPolicyofcache-first, which we can achieve by modifyingoptions.fetchPolicyinQueryManager#fetchQueryObservablefor the nextfetchQueryObservablecall, using the same options object that theReobserveralways passes tofetchQueryObservable.Note: if these
FetchPolicytransitions get much more complicated, we might consider using some sort of state machine to capture the transition rules.Should fix #6305, and a few other related issues (TBD).