fix: networkStatus update Issue in useSuspenseQuery with cache-and-network policy

[gronxb]

This PR resolves a bug where useSuspenseQuery with a cache-and-network fetch policy does not update its networkStatus when there's a concurrent useReadQuery with the same fetch policy.

There is no test code in the cache folder, so I was unable to write a test, please advise if this is necessary.

I am attaching a reproduction of the error instead of the test code.

The expected result is that all networkStatus should be 7 (networkStatus.ready).
repro codesandbox

Bug Repro

function PersonList() {
  const { data, networkStatus } = useSuspenseQuery(ALL_PEOPLE, {
    fetchPolicy: "cache-and-network",
    variables: {},
  });

  return (
    <div>


      {/* Expected result:7, actual result: 1 */}
      <p>networkStatus: {networkStatus}</p>


      <ul>
        {data.people.map((person) => (
          <li key={person.id}>{person.name}</li>
        ))}
      </ul>
    </div>
  );
}

function App() {
  const [queryRef] = useBackgroundQuery(ALL_PEOPLE, {
    fetchPolicy: "cache-and-network",
  });
  const { data, networkStatus } = useReadQuery(queryRef);

  return (
    <main>
      <h2>Background Query & Read Query</h2>
      <p>networkStatus: {networkStatus}</p>
      {data?.people.map((person) => (
        <li key={person.id}>{person.name}</li>
      ))}

      <h2>Suspense Query</h2>
      <Suspense fallback={<p>Loading…</p>}>
        <PersonList />
      </Suspense>
    </main>
  );
}

Checklist:

• If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
• 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

[netlify]

‼️ Deploy request for apollo-client-docs rejected.

Name	Link
🔨 Latest commit	8f59144

[changeset-bot]

🦋 Changeset detected

Latest commit: 1d84f5d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[jerelmiller]

Hey @gronxb 👋

Appreciate the PR! I'd like to dig into this a bit more, but you've made an interesting observation! Something I found curious was that if I remove the empty variables from useSuspenseQuery, the networkStatus behaves as expected:

i.e.

  const { data, networkStatus } = useSuspenseQuery(ALL_PEOPLE, {
    fetchPolicy: "cache-and-network",
-   variables: {},
  });

Judging based on your changes, I'm willing to bet this is less about useBackgroundQuery/useReadQuery in tandem with useSuspenseQuery and more about the fact that having a response return the same data as whats already in the cache while using cache-and-network means we skip the re-render, which would ensure the networkStatus is kept up-to-date. I believe you should be able to reproduce this same result by writing a result to the cache ahead of time for that query, then mounting a useSuspenseQuery.

As for tests, we don't have many on our QueryReference class itself right now but rather test it through our hooks tests themselves. For example, here are our useSuspenseQuery tests. I'd definitely love to see a test or two in place before I'm willing to accept this.

Appreciate you digging into this!

[gronxb]

@jerelmiller

Thanks for the advice, networkStatus is not updating when returning the same data, I've written test code for that.

[jerelmiller]

🎉 💯 🔥 Thanks so much for diving into this issue and fixing it! Very much appreciated! I had a minor change to your changeset to give a bit more detailed description, but this looks great! Thanks again!