Repository: apollographql/apollo-client

Problem statement:
`resetStore` fetches `watchQuery` that has never been subscribed to
**Intended outcome:**
Calling ApolloClient.resetStore() should not execute queries that are not actively subscribed to.

**Actual outcome:**
Calling ApolloClient.resetStore() causes a `watchQuery` that has been assigned to a variable but _never subscribed to_ to fetch data.

**How to reproduce the issue:**
Follow instructions on [my Apollo-Angular StackBlitz reproduction](https://stackblitz.com/edit/apollo-client-3-reset-store?file=app/list.component.ts).

**Versions**
```
  System:
    OS: macOS 10.15.6
  Binaries:
    Node: 12.18.4 - ~/.nvm/versions/node/v12.18.4/bin/node
    Yarn: 1.22.5 - ~/.nvm/versions/node/v12.18.4/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v12.18.4/bin/npm
  Browsers:
    Chrome: 86.0.4240.75
    Firefox: 81.0.1
    Safari: 14.0
  npmPackages:
    @apollo/client: 3.2.2 => 3.2.2 
    apollo-angular: 2.0.4 => 2.0.4 
    apollo-server-express: 2.18.1 => 2.18.1 
```
[3.0 beta-43]: reFetchObservableQueries causes skipped query to be fetched
We have some logic that checks which workspace a user is when navigating through our app:

```javascript
  const nodeId = UrlHelper.getNodeIdFromUrl(router);

  const { data: nodeData, error: nodeError, loading: nodeLoading } = useQuery<
    CurrentNodeQuery,
    CurrentNodeQueryVariables
  >(currentNodeQuery, {
    skip: !nodeId,
    variables: {
      id: IdConverter.encodeId(nodeId)
    },
    errorPolicy: 'all'
  });
```
If no workspace can be found, it's skipped.

If the user is anonymous and navigates elsewhere and decides to login, we clear the cache and refetch queries:

```javascript
      await client.clearStore();
     ... other stuff...
      await client.reFetchObservableQueries(false);
```

Doing this re-executes the query to check the workspace.  However, at that point the "skip" logic is ignored and the query fails because there is no workspace in the context, causing the workspace ID to be null.  Therefore we get the following error:

`"[GraphQL error]: Message: Field 'id' of variable 'id' has coerced Null value for NonNull type 'ID!', Location: [{"line":1,"column":19}], Path: undefined`

**Intended outcome:**
Ideally only non-skipped queries would be re-executed, or the skip logic would be re-run somehow (a callback function for "skip" perhaps ?).  This doesn't seem to be happening.

**Actual outcome:*

"reFetchObservableQueries" is refetching all queries, regardless of whether they should be skipped or not.

**How to reproduce the issue:**

I'm working on some reproduction code.

**Versions**

  System:
    OS: macOS 10.15.4
  Binaries:
    Node: 12.14.1 - ~/.nvm/versions/node/v12.14.1/bin/node
    Yarn: 1.22.0 - ~/.nvm/versions/node/v12.14.1/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v12.14.1/bin/npm
  Browsers:
    Chrome: 81.0.4044.92
    Firefox: 75.0
    Safari: 13.1
  npmPackages:
    @apollo/client: ^3.0.0-beta.43 => 3.0.0-beta.43 
    @apollo/link-batch-http: ^2.0.0-beta.3 => 2.0.0-beta.3 
    @apollo/link-context: ^2.0.0-beta.3 => 2.0.0-beta.3 
    @apollo/link-error: ^2.0.0-beta.3 => 2.0.0-beta.3 
    @apollo/react-common: ^4.0.0-beta.1 => 4.0.0-beta.1 
    @apollo/react-components: ^4.0.0-beta.1 => 4.0.0-beta.1 
    @apollo/react-hoc: ^4.0.0-beta.1 => 4.0.0-beta.1 
    @apollo/react-ssr: ^4.0.0-beta.1 => 4.0.0-beta.1 
    apollo-link-log: ^1.1.3 => 1.1.3 
    apollo-server: ^2.11.0 => 2.11.0 
    react-apollo: ^3.1.3 => 3.1.3 




Hints:

This is affecting me as well, I arrived at the same conclusion as op, reFetchObservableQueries is not respecting the skip options.

[The official guide here](https://www.apollographql.com/docs/react/v3.0-beta/networking/authentication/#reset-store-on-logout) recommends calling `client.resetStore()` on user logout, but it will unintentionally cause all skipped query to fire due to this bug. 

possibly related issues:
https://github.com/apollographql/apollo-client/issues/6342
https://github.com/apollographql/apollo-client/issues/6190
https://github.com/apollographql/react-apollo/issues/3492
Yep, broken. See https://github.com/apollographql/react-apollo/issues/3492#issuecomment-644385154.
I noticed that `client.resetStore()` is also executing skipped queries, which is very problematic. See: https://github.com/apollographql/apollo-client/issues/7113
Appears to have been caused by #6999 which was intended to fix #6796

What I don't understand is the use case/motivation behind #6796 in the first place. Why would you want to `refetch` a skipped query? If it's a workaround due to the mentioned `useLazyQyery` limitation, it seems wrong to accommodate that via `useQuery`.

Patch:
diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
--- a/src/core/ObservableQuery.ts
+++ b/src/core/ObservableQuery.ts
@@ -620,6 +620,10 @@ once, rather than every time you call fetchMore.`);
     },
   };
 
+  public hasObservers() {
+    return this.observers.size > 0;
+  }
+
   private tearDownQuery() {
     const { queryManager } = this;
 
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
--- a/src/core/QueryManager.ts
+++ b/src/core/QueryManager.ts
@@ -563,7 +563,7 @@ export class QueryManager<TStore> {
     const observableQueryPromises: Promise<ApolloQueryResult<any>>[] = [];
 
     this.queries.forEach(({ observableQuery }, queryId) => {
-      if (observableQuery) {
+      if (observableQuery && observableQuery.hasObservers()) {
         const fetchPolicy = observableQuery.options.fetchPolicy;
 
         observableQuery.resetLastResults();


Test patch:
diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts
--- a/src/core/__tests__/QueryManager/index.ts
+++ b/src/core/__tests__/QueryManager/index.ts
@@ -3506,9 +3506,18 @@ describe('QueryManager', () => {
           }
         }
       `;
-
-      const queryManager = mockQueryManager(reject);
+      const data = {
+        author: {
+          firstName: 'John',
+          lastName: 'Smith',
+        },
+      };
+      const queryManager = mockQueryManager(reject, {
+        request: { query },
+        result: { data }
+      });
       const obs = queryManager.watchQuery<any>({ query });
+      obs.subscribe({});
       obs.refetch = resolve as any;
 
       queryManager.resetStore();
@@ -3536,6 +3545,7 @@ describe('QueryManager', () => {
       let refetchCount = 0;
 
       const obs = queryManager.watchQuery(options);
+      obs.subscribe({});
       obs.refetch = () => {
         ++refetchCount;
         return null as never;
@@ -3570,6 +3580,41 @@ describe('QueryManager', () => {
 
       let refetchCount = 0;
 
+      const obs = queryManager.watchQuery(options);
+      obs.subscribe({});
+      obs.refetch = () => {
+        ++refetchCount;
+        return null as never;
+      };
+
+      queryManager.resetStore();
+
+      setTimeout(() => {
+        expect(refetchCount).toEqual(0);
+        resolve();
+      }, 50);
+    });
+
+    itAsync('should not call refetch on a non-subscribed Observable if the store is reset', (resolve, reject) => {
+      const query = gql`
+        query {
+          author {
+            firstName
+            lastName
+          }
+        }
+      `;
+
+      const queryManager = createQueryManager({
+        link: mockSingleLink().setOnError(reject),
+      });
+
+      const options = {
+        query,
+      } as WatchQueryOptions;
+
+      let refetchCount = 0;
+
       const obs = queryManager.watchQuery(options);
       obs.refetch = () => {
         ++refetchCount;
@@ -3906,10 +3951,19 @@ describe('QueryManager', () => {
           }
         }
       `;
-
-      const queryManager = mockQueryManager(reject);
+      const data = {
+        author: {
+          firstName: 'John',
+          lastName: 'Smith',
+        },
+      };
+      const queryManager = mockQueryManager(reject, {
+        request: { query },
+        result: { data },
+      });
 
       const obs = queryManager.watchQuery({ query });
+      obs.subscribe({});
       obs.refetch = resolve as any;
 
       queryManager.reFetchObservableQueries();
@@ -3937,6 +3991,7 @@ describe('QueryManager', () => {
       let refetchCount = 0;
 
       const obs = queryManager.watchQuery(options);
+      obs.subscribe({});
       obs.refetch = () => {
         ++refetchCount;
         return null as never;
@@ -3972,6 +4027,7 @@ describe('QueryManager', () => {
       let refetchCount = 0;
 
       const obs = queryManager.watchQuery(options);
+      obs.subscribe({});
       obs.refetch = () => {
         ++refetchCount;
         return null as never;
@@ -4007,6 +4063,7 @@ describe('QueryManager', () => {
       let refetchCount = 0;
 
       const obs = queryManager.watchQuery(options);
+      obs.subscribe({});
       obs.refetch = () => {
         ++refetchCount;
         return null as never;
@@ -4021,6 +4078,40 @@ describe('QueryManager', () => {
       }, 50);
     });
 
+    itAsync('should not call refetch on a non-subscribed Observable', (resolve, reject) => {
+      const query = gql`
+        query {
+          author {
+            firstName
+            lastName
+          }
+        }
+      `;
+
+      const queryManager = createQueryManager({
+        link: mockSingleLink().setOnError(reject),
+      });
+
+      const options = {
+        query
+      } as WatchQueryOptions;
+
+      let refetchCount = 0;
+
+      const obs = queryManager.watchQuery(options);
+      obs.refetch = () => {
+        ++refetchCount;
+        return null as never;
+      };
+
+      queryManager.reFetchObservableQueries();
+
+      setTimeout(() => {
+        expect(refetchCount).toEqual(0);
+        resolve();
+      }, 50);
+    });
+
     itAsync('should NOT throw an error on an inflight query() if the observed queries are refetched', (resolve, reject) => {
       let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`