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

fix(composable): Remove immediate result logic #1388

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

ishitatsuyuki
Copy link
Contributor

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: #1315

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 Author

Could @Akryum or someone take a look at this?

@ishitatsuyuki
Copy link
Contributor Author

@Akryum Could you look into this? A user reported that this fixes their issue at #1315 (comment).

@martijnvanloonafs
Copy link

martijnvanloonafs commented Oct 5, 2022

Thanks a lot for this, I have the same issue in vue apollo 3. I found this code over there that looked similar. After removing it "cache-and-network" works as expected (show cache, start network request in background) Before it just did a request and ignored cache.

      if (this.options.fetchPolicy !== 'no-cache' || this.options.notifyOnNetworkStatusChange) {
        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.nextResult(currentResult);
        }
      }

To use this in your project you can edit the file and then run this:

npx patch-package vue-apollo

and add this to the scripts section of your package.json
"postinstall": "npx patch-package"

edit: Ran into issues with $apollo.loading. Replacing the previously mentioned code with the following works and keeps $apollo.loading intact.

      var currentResult = this.observer.getCurrentResult(false);

      if (!currentResult.data) {
        if (this.options.fetchPolicy !== 'no-cache' || this.options.notifyOnNetworkStatusChange) {
          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.nextResult(currentResult);
          }
        }
      }

@ishitatsuyuki
Copy link
Contributor Author

@Akryum or someone, could you take a look? Don't really want to hard fork something under vuejs umbrella.

@Akryum
Copy link
Member

Akryum commented Oct 5, 2022

I guess this could break apps with Apollo Client 2.

@ishitatsuyuki
Copy link
Contributor Author

Even looking at the now defunct https://github.com/apollographql/react-apollo, it doesn't seem that this logic has existed from the first place. Is Apollo Client 2 compatibility a concern large enough for you? So far, this seems to have fixed stuff for people using both Vue Apollo 3 and 4.

@Akryum Akryum changed the title fix(composable): Remove bogus logic querying for immediate result fix(composable): Remove immediate result logic Oct 5, 2022
@Akryum Akryum merged commit fc98307 into vuejs:v4 Oct 5, 2022
@ishitatsuyuki ishitatsuyuki deleted the usequery-stale branch October 5, 2022 13:38
@Akryum
Copy link
Member

Akryum commented Feb 3, 2023

Looks like this is introducing hydration mismatch errors when doing SSR because the data is not available to the component yet when mounting :(

@Akryum
Copy link
Member

Akryum commented Feb 3, 2023

#1432

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

Successfully merging this pull request may close these issues.

None yet

3 participants