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

FetchPolicy cache-and-network always uses networkStatus = 1 regardless of cache hit #3660

Closed
nrdobie opened this issue Jul 5, 2018 · 28 comments

Comments

@nrdobie
Copy link

nrdobie commented Jul 5, 2018

Intended outcome:

This is the issue I originally found but it appears that the issue is with the client and not the Query component. apollographql/react-apollo#1217

When using the cache-and-network fetch policy I should be able to identify when I have data from the cache and when it is first retrieving the data.

<Query query={gql`{ now }`  fetchPolicy="cache-and-network" notifyOnNetworkStatusChange>
  {(data, error, networkStatus) => (
    if (networkStatus < 4) return 'Loading'
    if (error) return 'Error!'

    return <div>
      <time>data.now</time>
      {networkStatus === 4 && 'Refeshing'}
    </div>
  )}
</Query>

The query { now } returns the current date and time.

I would expect networkStatus to be 1 when it is making the first request, but 4 on each subsequent test as it is refetching the data.

Actual outcome:

Today the first request and subsequent requests come in using networkStatus = 1 making it difficult to identify the difference between the two states.

Versions

System:
OS: macOS Sierra 10.12.6
Binaries:
Node: 8.9.3 - ~/.nvm/versions/node/v8.9.3/bin/node
Yarn: 1.6.0 - ~/.nvm/versions/node/v8.9.3/bin/yarn
npm: 6.1.0 - ~/.nvm/versions/node/v8.9.3/bin/npm
Browsers:
Chrome: 67.0.3396.99
Firefox: 61.0
Safari: 11.1

@dallonf
Copy link

dallonf commented Jul 8, 2018

This is quite similar to an issue I raised a while ago - I actually wrote a PR to address this, but it got lost in 2.0 limbo. It's definitely important to solve!

@rtymchyk
Copy link

For anyone dealing with this PITA bug, an alternative is to use network-only fetchPolicy. For whatever reason it functions exactly the same as cache-then-network in my case, but reports correct network statuses.

@frederikhors
Copy link

@rtymchyk are you sure? With network-only the cache is not considered, or it is bugged?

@ALL any news on this bug (FetchPolicy cache-and-network always uses networkStatus = 1 regardless of cache hit)?

@rtymchyk
Copy link

@frederikhors The network fetch happens with an explicit refetch only, which works well enough for me. Don't see much of an alternative 😢

@frederikhors
Copy link

@dallonf can you make a new smaller PR just for this problem in 2.0 land?

@dallonf
Copy link

dallonf commented Sep 17, 2018

I'm not an Apollo maintainer, just a guy who wrote a (declined) PR once

@frederikhors
Copy link

@dallonf yes, I know. But maybe in that period there was a mess with 2.0 coming. Can you just re-write a smaller PR just for fix this? When you done I personally commit my time to assure it passes.

@dallonf
Copy link

dallonf commented Sep 17, 2018

I could try, but I'm no more qualified than anybody else looking at this issue, and I don't have a lot of time to contribute to OSS at the moment, so if you really need this fixed ASAP, you're probably going to have to write the PR yourself.

It's also worth noting that my original PR(#1607) isn't a "fix" - I diagnosed the problem as a gap in the design and added a whole new API to distinguish between two currently ambiguous states. This is something I really wouldn't be comfortable committing a lot of time developing until I have confirmation from the maintainers that this is a PR they would support; I don't want it to get ignored until the whole codebase changes out from under it again.

@vladshcherbin
Copy link

Same thing with broken cache-and-network and variable changes, always 1 instead of 2.

Somehow it works for me with network-only which changes to 2 and still shows stale data in a component. 🤦‍♂️ 😆

@jsslai
Copy link

jsslai commented Oct 17, 2018

Can someone confirm how this should work? I mean different network policies and how they affect response data and network status?

@craigmulligan
Copy link

I'm seeing a similar issue. Basically, I make a query with cache-and-network it returns it from the cache and sets the networkStatus to 7. It then makes the remote request and so the networkStatus is set to 1.

I tried to solve the issue by manually running markQueryResult() for the said queries but they still didn't update. It seems to be some disconnect between the query manager and react-apollo.

I have a feeling this is the offending code As you can see the promise isn't return like the other fetch-policies. I tried a few things but couldn't get the networkStatus to update.

Any ideas?

@elitan
Copy link

elitan commented Nov 24, 2018

Same thing with broken cache-and-network and variable changes, always 1 instead of 2.

Somehow it works for me with network-only which changes to 2 and still shows stale data in a component.

For me, the networkStatus returns same values for both cache-and-network and network-only.

Hopa this bug gets addressed soon. A comment from the maintainers would be nice just confirming it is a bug?!

@wilcoschoneveld
Copy link

I have the same issue. Only way to distinguish a cache hit is to use something like:

if (!data || (loading && !data.documents)) {
    return <CardLoader />;
}

This is suboptimal because I cannot re-use this logic across different Query-components. Would prefer to have something like networkStatus=4.

@hwillson any comments? Thanks!

@cem2ran
Copy link

cem2ran commented Dec 25, 2018

Would be great to resolve this issue by either acknowledging that this is a bug and will be fixed, such that there's only a loading state when our cache is empty or introduce a non-breaking additive "cache-then-network" policy which does the above.

@shelmire
Copy link
Contributor

shelmire commented Jan 6, 2019

Would be nice to see this resolved so we can have reusable logic.

@revskill10
Copy link

What's difference between cache-first and cache-and-network though ? These policies seems identical to me, so if it's better to remove the latter for not confusing users ?

@rstrand
Copy link

rstrand commented Jan 30, 2019

@revskill10 With cache-first if the data is found in the cache then no network request is made. cache-and-network will return data from the cache but will also make a network request (even on a cache hit) and return updated data if available.

@Emiliano-Bucci
Copy link

Emiliano-Bucci commented Feb 9, 2019

Is there a way (using cache-and-network) to do not update the cache if the data received after the network request is the same as the old one in the cache? Let's say i have a blog: the user enters in one article and it loads fast (since the data is in the cache); but if the autor of the artiche made some update, then apollo updates the relevant data, and react updates the relative component. Now it always update the cache, causing the unmounting and mounting again the various component, which is weird because the user sees a component with some data (the data from the cache) then it dissapear, and then appear again.

@MrLoh
Copy link

MrLoh commented Feb 22, 2019

It's a bit concerning that this issue has had no attention from the team at all in 8 months.

A workaround to check wether data has been resolved from cache is to use isEmpty(data) from lodash or something similar.

@benjamn benjamn self-assigned this Apr 30, 2019
@benjamn benjamn added this to the Release 2.6.0 milestone Apr 30, 2019
benjamn added a commit that referenced this issue Apr 30, 2019
Blindly setting networkStatus to NetworkStatus.loading after the client
part of a cache-and-network query was destroying useful information about
the status of the request. Instead, we should use NetworkStatus.ready only
when the request is complete, and leave networkStatus untouched otherwise.

Should fix #1217.
Should fix #3660.
Should fix #4693.
benjamn added a commit that referenced this issue Apr 30, 2019
Blindly setting networkStatus to NetworkStatus.loading after the client
part of a cache-and-network query was destroying useful information about
the status of the request. Instead, we should use NetworkStatus.ready only
when the request is complete, and leave networkStatus untouched otherwise.

Fixes #3660, #4693, and apollographql/react-apollo#1217.
@benjamn
Copy link
Member

benjamn commented May 21, 2019

We just published the final version of apollo-client@2.6.0 to npm, including the fix for this issue! See CHANGELOG.md if you're curious about the other changes in this release.

@antiold
Copy link

antiold commented May 22, 2019

Still not working for me. I do not think cache-and-network is refetching therefore networkStatus is never 4.

Edit 1:

In a quick glance this should be be true if fetchPolicy is cache-and-network and there is something in the cache, no?

@thompk2
Copy link

thompk2 commented May 24, 2019

I'm running into the same issue as @antiold. with apollo-client@2.6.0 I see networkStatus updating to in progress on initial load, but networkStatus still isn't updating on a refetch

@TheoMer
Copy link

TheoMer commented Aug 3, 2019

I can confirm that this is still an issue for me with Apollo-client 2.6.3 and a fetchPolicy of cache-and-network.

@brianbolnick
Copy link

This is an issue for us as well. It looks like refetch is trying to pull from the cache with cache-and-network set as the fetch-policy. We were able to temporarily able to fix this by adding a dummy variable to the refetch

@kylesuss
Copy link

kylesuss commented Sep 23, 2019

@benjamn I have created a simplified reproduction of this problem on Apollo client 2.6.4, which I think is still an issue. See:

https://codesandbox.io/s/apollo-cache-network-issue-miren

Notice that when you toggle the component which has the query back to visibility, it always goes from networkStatus 1 -> 7. Given the cache-and-network fetchPolicy is used, shouldn't we be seeing it start at 4 during the cache hit and subsequent refetch? My expectation is that when the query component is shown for the 2nd time, that it will have a network status of 4 until the request finishes.

It seems like the concept of "refetch" is being used to describe the issue here, but I think the Apollo concept of "refetch" is not really applicable (literally calling refetch). Instead, I think people are saying that when a query that has a cache-and-network fetch policy is run for a second time (potentially through a re-render), there is no way to tell that the query is cached through the networkStatus.

With that in mind, should I open a new issue?

@seeruk
Copy link

seeruk commented Apr 24, 2020

This is a bit of an old issue, but it seems like it has the most relevant discussion.

I'm working on something at the moment where the view updates and I'm using cache-and-network to keep the interface feeling fast, while also being kept up-to-date.

I'm really struggling to figure out how to show loading states when I don't have cached data, but no loading state when I got data from the cache and Apollo is fetching the fresh data in the background. The problem is, when I don't have cached data, I still have the previous data returned from the hook, so I can't react to not having data, and loading is always true whenever it's loading either fresh data, or data in the background - so I can't use that either.

The closest I've come is reacting to when data changes, then comparing the new data with the old data to see if it's different, and if it's the same when loading is true then it must be the previous data. But it seems to bug out sometimes.

IMO what would be ideal is a different state when loading after data has been returned from the cache, so you could check loading === true and some network status that is something like "fetchingInBackground" (or maybe something more concise...)

@renato
Copy link

renato commented Aug 14, 2020

I've forked @kylesuss reproduction using @apollo/client 3.1.3: https://codesandbox.io/s/apollo-cache-network-issue-ck662, the behavior is the same.

@puglyfe
Copy link

puglyfe commented Nov 23, 2021

I'm also running into this issue while trying to get setup with apollo-cache-persist. In the interest of a workaround that doesn't require refactoring the render logic of existing queries, I created a usePersistedCacheQuery hook that wraps useQuery and modifies the loading attribute to only return true if the query result doesn't already exist in the cache.

import { useQuery } from '@apollo/client';

const usePersistedCacheQuery = (query, options) => {
  const result = useQuery(query, {
    ...options,
    fetchPolicy: 'cache-and-network',
  });

  const isActuallyLoading =
    result.loading &&
    !result.client.readQuery({
      query,
      variables: options?.variables,
    });

  return { ...result, loading: isActuallyLoading };
};

export default usePersistedCacheQuery;

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests