"Loading" stays true when the same result is returned twice in Apollo Client 3.6.9

[michaelcbrook]

This has been a long-standing issue that I've seen numerous people bring up, but I may have found the cause.

Here is the problem:

const request = {
  query: gql`
    query test($foo: String) {
      test(foo: $foo) {
        value
      }
    }
  `,
  variables: { foo: "bar" },
  notifyOnNetworkStatusChange: true
};

const observer = client.watchQuery(request);

observer.subscribe({
  next: ({ loading }) => {
    console.log(loading);
  }
});

When the GraphQL query returns the same result successfully and subsequently, even if the variables have changed, the loading flag remains true even after the second request returns successfully. It does not return back to false until the result returns something different than it did the last time.

The expected outcome is that loading flips to true when the request is in flight and then flips back to false after the request completes, even if the result is the same between the two requests.

I believe there's a good chance this bug is due to this bit of code:

  private reportResult(
    result: ApolloQueryResult<TData>,
    variables: TVariables | undefined,
  ) {
    const lastError = this.getLastError();
    if (lastError || this.isDifferentFromLastResult(result)) {
      if (lastError || !result.partial || this.options.returnPartialData) {
        this.updateLastResult(result, variables);
      }

      iterateObserversSafely(this.observers, 'next', result);
    }
  }

I believe that maybe it was supposed to be:

  private reportResult(
    result: ApolloQueryResult<TData>,
    variables: TVariables | undefined,
  ) {
    const lastError = this.getLastError();
    if (lastError || !result.partial || this.options.returnPartialData || this.isDifferentFromLastResult(result)) {
      this.updateLastResult(result, variables);
      iterateObserversSafely(this.observers, 'next', result);
    }
  }

In the former, the observer will never iterate if the request is successful and the next result is the same as the previous result.

Maybe this was just a minor coding oversight. But it would be nice to get feedback from the Apollo Client team to make sure this aligns with their thinking.

The way I was able to confirm this was the issue is that, when I added a timestamp of the request to the GraphQL API result, the problem was completely gone. So when the result is different with each request, it works as intended. But I shouldn't have to do this in a production setting.

Note I am running @apollo/client 3.6.9 and graphql 16.6.0, both the newest versions. I am also not using React. So if a fix for this was previously introduced for React, I'm afraid it wouldn't have fixed the root of the problem in Apollo Client itself.

Many tickets about this have been opened by others, all with suggestions of different workarounds, like changing networkPolicy to network-only, no-cache, or cache-and-network. Or even setting pollInterval to 0. But none of these worked for me, presumably because the root problem seems to be related to the conditions under which the observer will iterate. But it is interesting that the cache is still checked even when using a networkPolicy of no-cache.

Here are some of the related issues I found on this topic:

#6334
#9845
#9844 (another attempted fix, but I wonder if this one was overkill)
#6417 (a merged PR from two years ago attempting to fix this, but not successfully)
#9689 (maybe related)
#9668 (also maybe related)

Would greatly appreciate any insight on this. I currently don't have any other workarounds other than to add a timestamp to our API, but since we have a public-facing API, this would not be ideal.

Thank you!

[hwillson]

Sorry to hear this is a problem @michaelcbrook - any chance you can provide a small runnable reproduction (repro template, repro codesandbox) that demonstrates the problem? This will help expedite a fix.

[michaelcbrook]

Hey @hwillson,

This one gave me a run for my money to cut down to a reproduction, but I finally got it:

https://codesandbox.io/s/competent-worker-7qu3p1?file=/src/index.jsx

Turns out there are some more specific criteria that trigger this bug. In working on the reproduction, I found this error only happens under these conditions:

1. Variables must contain non-primitive values (I confirmed an array of objects creates the issue)
2. The non-primitive value must change between requests
3. The next result must be the same as the previous result, even though the variables have changed

Of note, the array of objects is being passed as a JSON GraphQL type via the scalars provided by this package: https://www.npmjs.com/package/graphql-scalars

...but that GraphQLJSON definition is set on the server, not the client...

I have not yet tested whether or not this issue still happens with a natively provided data type instead of a custom scalar, but seeing as though Apollo Client should be naive to that type (I think), that may not have any bearing on the problem, at least in my mind.

But I wanted to get your feedback and see whether that's consistent with your thinking or not.

As for my proposed solution, I'm less certain now in light of what I found after creating this reproduction. But that's beyond my knowledge.

Thanks!

[dpintos1-rbi]

In case this can help, in our team we were able to replicate this issue by making two parallel async calls with the same params, as follow:

 LOG  filter: NEARBY
 LOG  input: {"coordinates": {"userLat": 25, "userLng": -80.1}, "first": 20}
 LOG  filter: FAVORITE
 LOG  input: {"coordinates": {"userLat": 25, "userLng": -80.1}, "first": 20}

The GraphQL query looks something like:

gql`
   query GetPlaces($input: PlacesInput) {
 places(input: $input) {
   pageInfo {
     hasNextPage
     endCursor
   }
   totalCount
   nodes {
     ...PlaceNodeFragment
   }
 }
}
   ${PlaceNodeFragmentDoc}`;

[carlosbaraza]

We faced the same problem with the following case, which completely broke the cache and the entire SSR flow, which depends on extracting the cache and rehydrating it later. Really nasty bug to track down.

We have a main query that does:

query ArticlePage($slug: String!) {
  articles(
    where: { slug: { _eq: $slug } }
  ) {
    id
    ...
    linked_experts { // this is the resolver that breaks the cache
      expert {
        id
        ...
      }
    }
    ...
  }

The actual query that breaks the cache, not sure exactly why. I believe it is :

query SimilarArticles($args: SuggestArticlesInput!) {
  suggestArticles(args: $args) {
    similarity
    article_id
    suggestedArticle {
      id
      ...
      linked_experts {
        expert {
          title
        }
      }
    }
  }
}

The type definitions live in another graphql file:

type Query {
  suggestArticles(
    args: SuggestArticlesInput!
  ): [SuggestArticlesOutput!]!
}

input SuggestArticlesInput {
  companyIds: [uuid!]
  expertIds: [uuid!]
  limit: Int
}

type SuggestArticlesOutput {
  similarity: Float!
  article_id: uuid!
}

Hopefully this can give you extra information to find exactly what the bug is. In our case, it seems like fetching the linked_experts in the second query with an input type for the args broke the cache extraction and rehydration.

[bignimbus]

Looking at the codesandbox above:

When the Apollo Client version is set to 3.5.9:

When the Apollo Client version is set to 3.5.8:

[alessbell]

Hi @michaelcbrook 👋 Thanks for the reproduction here! After looking at your codesandbox, I was able to narrow it down a bit further: when the variables passed to observer.refetch change (and as you'll see in the failing test, this can be a single primitive value), but the resolved data does not, it results in a hanging loading: true (as you and some of the linked issues above described).

I've pushed up a failing test and some in-progress work to branch issue-10105-loading-stays-true. We may be able to call iterateObserversSafely if we know the variables have changed, but I'm still investigating whether this is a breaking change. I should be able open a PR shortly, but please let me know if my slightly altered reproduction doesn't align with what you're seeing. Thanks again!

[alessbell]

The fix was just merged and will go out in an upcoming patch release (3.7.1). Thanks all!

[alessbell]

The fix has been published in v3.7.1 and can be installed with npm i @apollo/client@next 👍

[michaelcbrook]

Awesome!! Thank you! Can't wait to try it out!