On reset store and allow ignore fragments on removeDirectives

packages/apollo-cache-inmemory/src/__tests__/cache.ts

@@ -6,15 +6,13 @@ import { InMemoryCache, ApolloReducerConfig, NormalizedCache } from '..';
 disableFragmentWarnings();
 
 describe('Cache', () => {
-  function createCache(
-    {
-      initialState,
-      config,
-    }: {
-      initialState?: any;
-      config?: ApolloReducerConfig;
-    } = {},
-  ): ApolloCache<NormalizedCache> {
+  function createCache({
+    initialState,
+    config,
+  }: {
+    initialState?: any;
+    config?: ApolloReducerConfig;
+  } = {}): ApolloCache<NormalizedCache> {
     return new InMemoryCache(
       config || { addTypename: false },
       // XXX this is the old format. The tests need to be updated but since it is mapped down

packages/apollo-client/CHANGELOG.md

@@ -5,6 +5,8 @@
 - Map coverage to original source
 - Added `getCacheKey` function to the link context for use in state-link [PR#2998](https://github.com/apollographql/apollo-client/pull/2998)
 - Fix Memory Leak in Query Manager [PR#3119](https://github.com/apollographql/apollo-client/pull/3119)
+- onResetStore callbacks occur before refetching Observable Queries[PR#3010](https://github.com/apollographql/apollo-client/pull/3010)
+- Error message for in flight queries during `resetStore` includes link completion note[PR#3010](https://github.com/apollographql/apollo-client/pull/3010)
 
 ### 2.2.3
 - dependency updates

packages/apollo-client/src/ApolloClient.ts

@@ -389,14 +389,19 @@ export default class ApolloClient<TCacheShape> implements DataProxy {
    * re-execute any queries then you should make sure to stop watching any
    * active queries.
    */
-  public resetStore(): Promise<ApolloQueryResult<any>[]> | Promise<null> {
+  public resetStore(): Promise<ApolloQueryResult<any>[] | null> {
     return Promise.resolve()
       .then(() => {
-        this.queryManager
-          ? this.queryManager.resetStore()
+        return this.queryManager
+          ? this.queryManager.clearStore()
           : Promise.resolve(null);
       })
-      .then(() => Promise.all(this.resetStoreCallbacks.map(fn => fn())));
+      .then(() => Promise.all(this.resetStoreCallbacks.map(fn => fn())))
+      .then(() => {
+        return this.queryManager
+          ? this.queryManager.reFetchObservableQueries()
+          : Promise.resolve(null);
+      });
   }
 
   /**

packages/apollo-client/src/__tests__/client.ts

@@ -2203,7 +2203,7 @@ describe('client', () => {
       cache: new InMemoryCache(),
     });
     client.queryManager = {
-      resetStore: () => {
+      clearStore: () => {
         done();
       },
     } as QueryManager;
@@ -2268,6 +2268,104 @@ describe('client', () => {
     expect(count).toEqual(2);
   });
 
+  it('invokes onResetStore callbacks before notifying queries during resetStore call', async () => {
+    const delay = time => new Promise(r => setTimeout(r, time));
+
+    const query = gql`
+      query {
+        author {
+          firstName
+          lastName
+        }
+      }
+    `;
+
+    const data = {
+      author: {
+        __typename: 'Author',
+        firstName: 'John',
+        lastName: 'Smith',
+      },
+    };
+
+    const data2 = {
+      author: {
+        __typename: 'Author',
+        firstName: 'Joe',
+        lastName: 'Joe',
+      },
+    };
+
+    let timesFired = 0;
+    const link = ApolloLink.from([
+      new ApolloLink(
+        () =>
+          new Observable(observer => {
+            timesFired += 1;
+            observer.next({ data });
+            return;
+          }),
+      ),
+    ]);
+
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+    });
+
+    let count = 0;
+    const onResetStoreOne = jest.fn(async () => {
+      expect(count).toEqual(0);
+      await delay(10).then(() => count++);
+      expect(count).toEqual(1);
+    });
+
+    const onResetStoreTwo = jest.fn(async () => {
+      expect(count).toEqual(0);
+      await delay(11).then(() => count++);
+      expect(count).toEqual(2);
+
+      try {
+        console.log(client.readQuery({ query }));
+        fail('should not see any data');
+      } catch (e) {
+        expect(e.message).toMatch(/Can't find field/);
+      }
+
+      client.cache.writeQuery({ query, data: data2 });
+    });
+
+    client.onResetStore(onResetStoreOne);
+    client.onResetStore(onResetStoreTwo);
+
+    let called = false;
+    const next = jest.fn(async d => {
+      if (called) {
+        expect(onResetStoreOne).toHaveBeenCalled();
+      } else {
+        expect(d).toEqual(data);
+        called = true;
+      }
+    });
+
+    const observable = client
+      .watchQuery<any>({
+        query,
+        notifyOnNetworkStatusChange: false,
+      })
+      .subscribe({
+        next,
+        error: fail,
+        complete: fail,
+      });
+
+    expect(count).toEqual(0);
+    await client.resetStore();
+    expect(count).toEqual(2);
+    //watchQuery should only receive data twice
+    expect(next).toHaveBeenCalledTimes(2);
+  });
+
   it('has a reFetchObservableQueries method which calls QueryManager', done => {
     const client = new ApolloClient({
       link: ApolloLink.empty(),

packages/apollo-client/src/core/QueryManager.ts

@@ -798,15 +798,19 @@ export class QueryManager<TStore> {
     }
   }
 
-  public resetStore(): Promise<ApolloQueryResult<any>[]> {
+  public clearStore(): Promise<void> {
     // Before we have sent the reset action to the store,
     // we can no longer rely on the results returned by in-flight
     // requests since these may depend on values that previously existed
     // in the data portion of the store. So, we cancel the promises and observers
     // that we have issued so far and not yet resolved (in the case of
     // queries).
     this.fetchQueryPromises.forEach(({ reject }) => {
-      reject(new Error('Store reset while query was in flight.'));
+      reject(
+        new Error(
+          'Store reset while query was in flight(not completed in link chain)',
+        ),
+      );
     });
 
     const resetIds: string[] = [];
@@ -816,22 +820,22 @@ export class QueryManager<TStore> {
 
     this.queryStore.reset(resetIds);
     this.mutationStore.reset();
+
     // begin removing data from the store
-    const dataStoreReset = this.dataStore.reset();
+    const reset = this.dataStore.reset();
+    return reset;
+  }
 
+  public resetStore(): Promise<ApolloQueryResult<any>[]> {
     // Similarly, we have to have to refetch each of the queries currently being
     // observed. We refetch instead of error'ing on these since the assumption is that
     // resetting the store doesn't eliminate the need for the queries currently being
     // watched. If there is an existing query in flight when the store is reset,
     // the promise for it will be rejected and its results will not be written to the
     // store.
-    const observableQueryPromises: Promise<
-      ApolloQueryResult<any>
-    >[] = this.getObservableQueryPromises();
-
-    this.broadcastQueries();
-
-    return dataStoreReset.then(() => Promise.all(observableQueryPromises));
+    return this.clearStore().then(() => {
+      return this.reFetchObservableQueries();
+    });
   }
 
   private getObservableQueryPromises(

packages/apollo-client/src/core/__tests__/QueryManager/index.ts

@@ -4,7 +4,11 @@ import { assign } from 'lodash';
 import gql from 'graphql-tag';
 import { DocumentNode, ExecutionResult, GraphQLError } from 'graphql';
 import { ApolloLink, Operation, Observable } from 'apollo-link';
-import { InMemoryCache, ApolloReducerConfig } from 'apollo-cache-inmemory';
+import {
+  InMemoryCache,
+  ApolloReducerConfig,
+  NormalizedCacheObject,
+} from 'apollo-cache-inmemory';
 
 // mocks
 import mockQueryManager from '../../../__mocks__/mockQueryManager';
@@ -118,7 +122,7 @@ describe('QueryManager', () => {
     });
     return new Promise<{
       result: ExecutionResult;
-      queryManager: QueryManager;
+      queryManager: QueryManager<NormalizedCacheObject>;
     }>((resolve, reject) => {
       queryManager
         .mutate({ mutation, variables })
@@ -2442,7 +2446,7 @@ describe('QueryManager', () => {
         },
       };
 
-      const queryManager = new QueryManager({
+      const queryManager = new QueryManager<NormalizedCacheObject>({
         link: mockSingleLink(
           {
             request: { query, variables },
@@ -3007,7 +3011,7 @@ describe('QueryManager', () => {
     });
 
     it('should only refetch once when we store reset', () => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`
         query {
           author {
@@ -3067,7 +3071,7 @@ describe('QueryManager', () => {
     });
 
     it('should not refetch toredown queries', done => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       let observable: ObservableQuery<any>;
       const query = gql`
         query {
@@ -3115,7 +3119,7 @@ describe('QueryManager', () => {
     });
 
     it('should not error on queries that are already in the store', () => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`
         query {
           author {
@@ -3133,12 +3137,15 @@ describe('QueryManager', () => {
 
       let timesFired = 0;
       const link = ApolloLink.from([
-        () =>
-          new Observable(observer => {
-            timesFired += 1;
-            observer.next({ data });
-            return;
-          }),
+        new ApolloLink(
+          () =>
+            new Observable(observer => {
+              timesFired += 1;
+              observer.next({ data });
+              observer.complete();
+              return;
+            }),
+        ),
       ]);
       queryManager = createQueryManager({ link });
       const observable = queryManager.watchQuery<any>({
@@ -3150,13 +3157,27 @@ describe('QueryManager', () => {
       return observableToPromise(
         { observable, wait: 20 },
         result => {
-          expect(result.data).toEqual(data);
-          expect(timesFired).toBe(1);
-          setTimeout(queryManager.resetStore.bind(queryManager), 10);
+          try {
+            expect(result.data).toEqual(data);
+            expect(timesFired).toBe(1);
+          } catch (e) {
+            return fail(e);
+          }
+          setTimeout(async () => {
+            try {
+              await queryManager.resetStore();
+            } catch (e) {
+              fail(e);
+            }
+          }, 10);
         },
         result => {
-          expect(result.data).toEqual(data);
-          expect(timesFired).toBe(2);
+          try {
+            expect(result.data).toEqual(data);
+            expect(timesFired).toBe(2);
+          } catch (e) {
+            fail(e);
+          }
         },
       );
     });
@@ -3290,7 +3311,7 @@ describe('QueryManager', () => {
     });
 
     it('should throw an error on an inflight query() if the store is reset', done => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`
         query {
           author {
@@ -3423,7 +3444,7 @@ describe('QueryManager', () => {
     });
 
     it('should only refetch once when we refetch observable queries', () => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`
         query {
           author {
@@ -3483,7 +3504,7 @@ describe('QueryManager', () => {
     });
 
     it('should not refetch toredown queries', done => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       let observable: ObservableQuery<any>;
       const query = gql`
         query {
@@ -3531,7 +3552,7 @@ describe('QueryManager', () => {
     });
 
     it('should not error on queries that are already in the store', () => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`
         query {
           author {
@@ -3743,7 +3764,7 @@ describe('QueryManager', () => {
     });
 
     it('should NOT throw an error on an inflight query() if the observed queries are refetched', done => {
-      let queryManager: QueryManager;
+      let queryManager: QueryManager<NormalizedCacheObject>;
       const query = gql`
         query {
           author {
@@ -4355,7 +4376,7 @@ describe('QueryManager', () => {
       );
       const cache = new InMemoryCache();
 
-      const queryManager = new QueryManager({
+      const queryManager = new QueryManager<NormalizedCacheObject>({
         link,
         store: new DataStore(cache),
       });