Repository: apollographql/apollo-client

Problem statement:
FetchPolicy `cache-and-network` always uses networkStatus = 1 regardless of cache hit
<!--
  Thanks for filing an issue on Apollo Client!

  Please make sure that you include the following information to ensure that your issue is actionable.

  If you don't follow the template, your issue may end up being closed without anyone looking at it carefully, because it is not actionable for us without the information in this template.

  If you're filing a feature request, you do not need to follow the template, but please mark the feature box at the bottom and include a specific example in which that feature would be useful.
-->

**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:**
<!--
A description of what actually happened, including a screenshot or copy-paste of any related error messages, logs, or other output that might be related. Places to look for information include your browser console, server console, and network logs. Please avoid non-specific phrases like “didn’t work” or “broke”.
-->

Today the first request and subsequent requests come in using `networkStatus = 1` making it difficult to identify the difference between the two states.

**Versions**
<!--
Run the following command in your project directory, and paste its (automatically copied to clipboard) results here:

`npx envinfo@latest --preset apollo --clipboard`
-->
  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


<!--**Issue Labels**

While not necessary, you can help organize our issues by labeling this issue when you open it.  To add a label automatically, simply [x] mark the appropriate box below:

- [ ] has-reproduction
- [ ] feature
- [ ] docs
- [ ] blocking
- [ ] good first issue

To add a label not listed above, simply place `/label another-label-name` on a line by itself.
-->

FetchPolicy `cache-and-network` always uses networkStatus = 1 regardless of cache hit
<!--
  Thanks for filing an issue on Apollo Client!

  Please make sure that you include the following information to ensure that your issue is actionable.

  If you don't follow the template, your issue may end up being closed without anyone looking at it carefully, because it is not actionable for us without the information in this template.

  If you're filing a feature request, you do not need to follow the template, but please mark the feature box at the bottom and include a specific example in which that feature would be useful.
-->

**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:**
<!--
A description of what actually happened, including a screenshot or copy-paste of any related error messages, logs, or other output that might be related. Places to look for information include your browser console, server console, and network logs. Please avoid non-specific phrases like “didn’t work” or “broke”.
-->

Today the first request and subsequent requests come in using `networkStatus = 1` making it difficult to identify the difference between the two states.

**Versions**
<!--
Run the following command in your project directory, and paste its (automatically copied to clipboard) results here:

`npx envinfo@latest --preset apollo --clipboard`
-->
  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


<!--**Issue Labels**

While not necessary, you can help organize our issues by labeling this issue when you open it.  To add a label automatically, simply [x] mark the appropriate box below:

- [ ] has-reproduction
- [ ] feature
- [ ] docs
- [ ] blocking
- [ ] good first issue

To add a label not listed above, simply place `/label another-label-name` on a line by itself.
-->



Hints:
This is quite similar to an [issue I raised a while ago](https://github.com/apollographql/react-apollo/issues/447) - I actually [wrote a PR](https://github.com/apollographql/apollo-client/pull/1607) to address this, but it got lost in 2.0 limbo. It's definitely important to solve!
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. 
@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)?
@frederikhors The network fetch happens with an explicit refetch only, which works well enough for me. Don't see much of an alternative 😢 
@dallonf can you make a new smaller PR just for this problem in 2.0 land?
I'm not an Apollo maintainer, just a guy who wrote a (declined) PR once
@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.
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(https://github.com/apollographql/apollo-client/pull/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.
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. 🤦‍♂️  😆 
Can someone confirm how this should work? I mean different network policies and how they affect response data and network status?
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](https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/core/QueryManager.ts#L420) 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?
> 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?!
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!

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.
Would be nice to see this resolved so we can have reusable logic.
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 ?
@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.
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. 
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.
This is quite similar to an [issue I raised a while ago](https://github.com/apollographql/react-apollo/issues/447) - I actually [wrote a PR](https://github.com/apollographql/apollo-client/pull/1607) to address this, but it got lost in 2.0 limbo. It's definitely important to solve!
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. 
@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)?
@frederikhors The network fetch happens with an explicit refetch only, which works well enough for me. Don't see much of an alternative 😢 
@dallonf can you make a new smaller PR just for this problem in 2.0 land?
I'm not an Apollo maintainer, just a guy who wrote a (declined) PR once
@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.
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(https://github.com/apollographql/apollo-client/pull/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.
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. 🤦‍♂️  😆 
Can someone confirm how this should work? I mean different network policies and how they affect response data and network status?
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](https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/core/QueryManager.ts#L420) 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?
> 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?!
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!

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.
Would be nice to see this resolved so we can have reusable logic.
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 ?
@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.
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. 
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.

Patch:
diff --git a/packages/apollo-client/src/data/queries.ts b/packages/apollo-client/src/data/queries.ts
--- a/packages/apollo-client/src/data/queries.ts
+++ b/packages/apollo-client/src/data/queries.ts
@@ -151,13 +151,14 @@ export class QueryStore {
   }
 
   public markQueryResultClient(queryId: string, complete: boolean) {
-    if (!this.store || !this.store[queryId]) return;
-
-    this.store[queryId].networkError = null;
-    this.store[queryId].previousVariables = null;
-    this.store[queryId].networkStatus = complete
-      ? NetworkStatus.ready
-      : NetworkStatus.loading;
+    const storeValue = this.store && this.store[queryId];
+    if (storeValue) {
+      storeValue.networkError = null;
+      storeValue.previousVariables = null;
+      if (complete) {
+        storeValue.networkStatus = NetworkStatus.ready;
+      }
+    }
   }
 
   public stopQuery(queryId: string) {


Test patch:
diff --git a/packages/apollo-client/src/core/__tests__/fetchPolicies.ts b/packages/apollo-client/src/core/__tests__/fetchPolicies.ts
--- a/packages/apollo-client/src/core/__tests__/fetchPolicies.ts
+++ b/packages/apollo-client/src/core/__tests__/fetchPolicies.ts
@@ -21,6 +21,7 @@ import subscribeAndCount from '../../util/subscribeAndCount';
 import { withWarning } from '../../util/wrap';
 
 import { mockSingleLink } from '../../__mocks__/mockLinks';
+import { NetworkStatus } from '../networkStatus';
 
 const query = gql`
   query {
@@ -345,3 +346,111 @@ describe('no-cache', () => {
       });
   });
 });
+
+describe('cache-and-network', function() {
+  it('gives appropriate networkStatus for refetched queries', done => {
+    const client = new ApolloClient({
+      link: ApolloLink.empty(),
+      cache: new InMemoryCache(),
+      resolvers: {
+        Query: {
+          hero(_data, args) {
+            return {
+              __typename: 'Hero',
+              ...args,
+              name: 'Luke Skywalker',
+            };
+          },
+        },
+      },
+    });
+
+    const observable = client.watchQuery({
+      query: gql`
+        query FetchLuke($id: String) {
+          hero(id: $id) @client {
+            id
+            name
+          }
+        }
+      `,
+      fetchPolicy: 'cache-and-network',
+      variables: { id: '1' },
+      notifyOnNetworkStatusChange: true,
+    });
+
+    function dataWithId(id: number | string) {
+      return {
+        hero: {
+          __typename: 'Hero',
+          id: String(id),
+          name: 'Luke Skywalker',
+        },
+      };
+    }
+
+    subscribeAndCount(done, observable, (count, result) => {
+      if (count === 1) {
+        expect(result).toEqual({
+          data: void 0,
+          loading: true,
+          networkStatus: NetworkStatus.loading,
+          stale: true,
+        });
+      } else if (count === 2) {
+        expect(result).toEqual({
+          data: dataWithId(1),
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          stale: false,
+        });
+        return observable.setVariables({ id: '2' });
+      } else if (count === 3) {
+        expect(result).toEqual({
+          data: dataWithId(1),
+          loading: true,
+          networkStatus: NetworkStatus.setVariables,
+          stale: false,
+        });
+      } else if (count === 4) {
+        expect(result).toEqual({
+          data: dataWithId(2),
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          stale: false,
+        });
+        return observable.refetch();
+      } else if (count === 5) {
+        expect(result).toEqual({
+          data: dataWithId(2),
+          loading: true,
+          networkStatus: NetworkStatus.refetch,
+          stale: false,
+        });
+      } else if (count === 6) {
+        expect(result).toEqual({
+          data: dataWithId(2),
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          stale: false,
+        });
+        return observable.refetch({ id: '3' });
+      } else if (count === 7) {
+        expect(result).toEqual({
+          data: dataWithId(2),
+          loading: true,
+          networkStatus: NetworkStatus.setVariables,
+          stale: false,
+        });
+      } else if (count === 8) {
+        expect(result).toEqual({
+          data: dataWithId(3),
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          stale: false,
+        });
+        done();
+      }
+    });
+  });
+});