Repository: apollographql/apollo-client

Problem statement:
useQuery with fetchPolicy: network-only return cache data at first render when remount component
<!--
  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.

  **PLEASE NOTE:** Feature requests and non-bug related discussions are no longer managed in this repo. Feature requests should be opened in https://github.com/apollographql/apollo-feature-requests.
-->

I'm trying to use `@apollo/client@3.4` and find out behavior change of `useQuery` with `fetchPolicy: 'network-only'` between AC3.4 and AC3.3, not sure if it's a bug or intended behavior.

**Intended outcome:**
<!--
What you were trying to accomplish when the bug occurred, and as much code as possible related to the source of the problem.
-->

For example we render an `<A />` component, and query with `fetchPolicy: 'network-only'`:

```jsx
function A() {
  const { data, loading } = useQuery(query, { fetchPolicy: 'network-only' };
  return <Blablabla data={data} />
}
```

Then we, unmount `<A />`, and remount `<A />` again. e.g. route or state change.

I expect when we remount `<A />`, the initial `data` returned by `useQuery` should be `undefined`, and after data fetching, the data will be renewed. which is the behavior of AC3.3 .

**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”.
-->

When I try the above use case on AC3.4, the initial returned `data` in `<A />` on remount will be cache data, not `undefined`.

**How to reproduce the issue:**
<!--
If possible, please create a reproduction using https://github.com/apollographql/react-apollo-error-template and link to it here. If you prefer an in-browser way to create reproduction, try: https://codesandbox.io/s/github/apollographql/react-apollo-error-template

Instructions for how the issue can be reproduced by a maintainer or contributor. Be as specific as possible, and only mention what is necessary to reproduce the bug. If possible, try to isolate the exact circumstances in which the bug occurs and avoid speculation over what the cause might be.
-->

reproduction: https://codesandbox.io/s/fervent-tesla-sb8oe?file=/src/App.js

Reproduction steps:
1. At first the App will render `<A />`. There's a console.log in useEffect of `<A />` to record `data` value on mount so you can see console printing `data on mount: undefined`.
2. click link B to switch to `<B />`
3. click link A to remount `<A />`. In AC3.4 the console will printing full data, but in AC3.3 (or AC2) it will print undefined.

**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 10.15.3
  Binaries:
    Node: 12.16.3 - ~/.nvm/versions/node/v12.16.3/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v12.16.3/bin/npm
  Browsers:
    Chrome: 92.0.4515.159
    Firefox: 89.0
    Safari: 13.0.5
  npmPackages:
    @apollo/client: ^3.4.9 => 3.4.9
```


Hints:
Same here.

After upgrading from 3.3 to 3.4 this started happening. I checked that last 3.3.21 version worked as expected. Change behaviour started from 3.4.0 and still occurs in 3.4.9 release.

Patch:
diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
--- a/src/core/ObservableQuery.ts
+++ b/src/core/ObservableQuery.ts
@@ -207,21 +207,35 @@ export class ObservableQuery<
       networkStatus,
     } as ApolloQueryResult<TData>;
 
-    // If this.options.query has @client(always: true) fields, we cannot trust
-    // diff.result, since it was read from the cache without running local
-    // resolvers (and it's too late to run resolvers now, since we must return a
-    // result synchronously).
-    if (!this.queryManager.transform(this.options.query).hasForcedResolvers) {
+    const { fetchPolicy = "cache-first" } = this.options;
+    // The presence of lastResult means a result has been received and
+    // this.options.variables haven't changed since then, so its absence means
+    // either there hasn't been a result yet (so these policies definitely
+    // should skip the cache) or there's been a result but it was for different
+    // variables (again, skipping the cache seems right).
+    const shouldReturnCachedData = lastResult || (
+      fetchPolicy !== 'network-only' &&
+      fetchPolicy !== 'no-cache' &&
+      fetchPolicy !== 'standby'
+    );
+    if (
+      shouldReturnCachedData &&
+      // If this.options.query has @client(always: true) fields, we cannot
+      // trust diff.result, since it was read from the cache without running
+      // local resolvers (and it's too late to run resolvers now, since we must
+      // return a result synchronously).
+      !this.queryManager.transform(this.options.query).hasForcedResolvers
+    ) {
       const diff = this.queryInfo.getDiff();
 
       if (diff.complete || this.options.returnPartialData) {
         result.data = diff.result;
       }
+
       if (equal(result.data, {})) {
         result.data = void 0 as any;
       }
 
-      const { fetchPolicy = "cache-first" } = this.options;
       if (diff.complete) {
         // If the diff is complete, and we're using a FetchPolicy that
         // terminates after a complete cache read, we can assume the next


Test patch:
diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts
--- a/src/core/__tests__/ObservableQuery.ts
+++ b/src/core/__tests__/ObservableQuery.ts
@@ -2027,7 +2027,7 @@ describe('ObservableQuery', () => {
         });
 
         expect(observable.getCurrentResult()).toEqual({
-          data: dataOne,
+          data: undefined,
           loading: true,
           networkStatus: NetworkStatus.loading,
         });
@@ -2036,7 +2036,7 @@ describe('ObservableQuery', () => {
           if (handleCount === 1) {
             expect(subResult).toEqual({
               loading: true,
-              data: dataOne,
+              data: undefined,
               networkStatus: NetworkStatus.loading,
             });
           } else if (handleCount === 2) {
diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -523,6 +523,40 @@ describe('useQuery Hook', () => {
       expect(result.current.loading).toBe(false);
       expect(result.current.data).toEqual(mocks[1].result.data);
     });
+
+    it('should not use the cache when using `network-only`', async () => {
+      const query = gql`{ hello }`;
+      const mocks = [
+        {
+          request: { query },
+          result: { data: { hello: 'from link' } },
+        },
+      ];
+
+      const cache = new InMemoryCache();
+      cache.writeQuery({
+        query,
+        data: { hello: 'from cache' },
+      });
+
+      const { result, waitForNextUpdate } = renderHook(
+        () => useQuery(query, { fetchPolicy: 'network-only' }),
+        {
+          wrapper: ({ children }) => (
+            <MockedProvider mocks={mocks} cache={cache}>
+              {children}
+            </MockedProvider>
+          ),
+        },
+      );
+
+      expect(result.current.loading).toBe(true);
+      expect(result.current.data).toBe(undefined);
+
+      await waitForNextUpdate();
+      expect(result.current.loading).toBe(false);
+      expect(result.current.data).toEqual({ hello: 'from link' });
+    });
   });
 
   describe('polling', () => {