Skip to content
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

Cached result is not instantly returned when fetchPolicy is set to "cache-and-network" #1315

Open
MCYouks opened this issue Feb 5, 2022 · 8 comments

Comments

@MCYouks
Copy link

MCYouks commented Feb 5, 2022

Describe the bug
When settings the query options with { fetchPolicy: "cache-and-network" }, the cached result is not instantly returned.

Instead, it waits until the data is fetched from the server to return the result.

To Reproduce
Steps to reproduce the behavior:

  1. Setup useQuery with the following options { fetchPolicy: "cache-and-network" }
  2. Setup 2 query variables: variables1 and variables2
  3. Execute the query with variables1, then variables2, then variables1 again — e.g. const { result } = useQuery(query, variable1, { fetchPolicy: "cache-and-network" })
  4. On the second time you query with variables1, the change in result won't be instantaneous. Instead, it will wait for a few seconds before updating.

Expected behavior
The cached result should be returned instantly.

Versions
vue: "^3.2.16"
@vue/apollo-composable: "^4.0.0-alpha.16"
@apollo/client: "^3.5.8"

@Kakahn
Copy link

Kakahn commented Feb 25, 2022

We have identified the same issue.

In the meantime, for this to be corrected, it would seem that if we add

notifyOnNetworkStatusChange: true,

at the useQuery level it works. But impossible to put it at the level of the config of apollo in defaultOptions.

Wait and see :)

@productdevbook
Copy link

some problem

@tafelnl
Copy link

tafelnl commented May 18, 2022

notifyOnNetworkStatusChange: true does fix it for fetching the query the first time. But if your component refetches the query (because of reactive variables changing or whatever), it will have the same issue again. So it's a workaround for some simple use cases, but definitely not for all use cases (and it's pretty common to use reactive variables in queries).

ishitatsuyuki added a commit to ishitatsuyuki/apollo that referenced this issue Jul 24, 2022
This old piece of code tried to get the result immediately by calling
getCurrentResult(), except that the function will also set the internal "last"
result if called without a `false` argument. This prevents `observer.next`
from being called with the same initial value.

The old code also conditionally ignored the result it just grabbed, and as a
result the initial value ended up not being passed on to the application.

The official Apollo React implementation does not have such special casing
and simply grabs the result whenever `observer.next` is called. Align our
code to that.

Issue: vuejs#1315
@ishitatsuyuki
Copy link
Contributor

If you're affected by this, could you please try #1388? You can setup linking manually or put "@vue/apollo-composable": "npm:@ishitatsuyuki/apollo-composable@4.0.0-alpha.19.usequery-stale" in your package.json. Let me know if it fix (or does not fix) your issues!

@xvaara
Copy link

xvaara commented Sep 15, 2022

@ishitatsuyuki fixes the issue for me.

@tafelnl
Copy link

tafelnl commented Oct 12, 2022

Heads up: there's now a release that ships the fix of @ishitatsuyuki https://github.com/vuejs/apollo/releases/tag/v4.0.0-beta.1

@Narretz
Copy link

Narretz commented Dec 3, 2022

This is fixed in vue-apollo 4.x, but the same logic exists in vue-apollo 3.x, see:

if (this.options.fetchPolicy !== 'no-cache' || this.options.notifyOnNetworkStatusChange) {
const currentResult = this.retrieveCurrentResult()
if (this.options.notifyOnNetworkStatusChange ||
// Initial call of next result when it's not loading (for Apollo Client 3)
(this.observer.getCurrentResult && !currentResult.loading)) {
this.nextResult(currentResult)
}
}

Removing the code also fixes the issue, but I found that at least in 3.x, the code does something.

Consider the following:

  • parent component executes query against network, and then renders child component
  • child component executes a query against cache-first

with the above code, the child component has the cached data before the component tries to render the template. Without the code, it doesn't (and you have to use v-if etc. to wait for the data)

This brings me to the point that vue-apollo is still not well tested.

  • the pull request was merged without an accompanying test case. A bug fix should always have a test case
  • the code that was removed apparentely wasn't covered by a test case either (but this may be because it was copied from the 3.x branch).
  • a common use case such as cache-and-network apparently wasn't tested either

As long as the testing strategy is subpar, errors like this will pop up again and again.

@viljark
Copy link

viljark commented Oct 31, 2023

I ended up patching the @vue/apollo-option directly inside vue-apollo-option.esm.js for now, I'm not 100% sure of the logic, but it solved the issue for us with "cache-and-network" not returning any cached results. Also network-only and cache-only seemed to work as expected.

diff --git a/node_modules/@vue/apollo-option/dist/vue-apollo-option.esm.js b/node_modules/@vue/apollo-option/dist/vue-apollo-option.esm.js
index e5a7f2f..efb1d95 100644
--- a/node_modules/@vue/apollo-option/dist/vue-apollo-option.esm.js
+++ b/node_modules/@vue/apollo-option/dist/vue-apollo-option.esm.js
@@ -783,7 +783,8 @@ var SmartQuery = /*#__PURE__*/function (_SmartApollo) {
         var currentResult = this.retrieveCurrentResult();
         if (this.options.notifyOnNetworkStatusChange ||
         // Initial call of next result when it's not loading (for Apollo Client 3)
-        this.observer.getCurrentResult && !currentResult.loading) {
+          (this.observer.getCurrentResult && this.options.fetchPolicy !== 'network-only') ||
+          (this.observer.getCurrentResult && this.options.fetchPolicy === 'network-only' && !currentResult.loading)) {
           this.nextResult(currentResult);
         }
       }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants