Fixed onComplete for FDC

.changeset/spotty-shirts-design.md

@@ -0,0 +1,5 @@
+---
+"@firebase/data-connect": patch
+---
+
+Fixed issue where onComplete wasn't triggering when the user calls `unsubscribe` on a subscription.

packages/data-connect/src/api.browser.ts

@@ -102,6 +102,7 @@ export function subscribe<Data, Variables>(
   return ref.dataConnect._queryManager.addSubscription(
     ref,
     onResult,
+    onComplete,
     onError,
     initialCache
   );

packages/data-connect/src/api/query.ts

@@ -45,6 +45,7 @@ export type QueryUnsubscribe = () => void;
 export interface DataConnectSubscription<Data, Variables> {
   userCallback: OnResultSubscription<Data, Variables>;
   errCallback?: (e?: DataConnectError) => void;
+  onCompleteCallback?: () => void;
   unsubscribe: () => void;
 }
 

packages/data-connect/src/core/QueryManager.ts

@@ -17,6 +17,7 @@
 
 import {
   DataConnectSubscription,
+  OnCompleteSubscription,
   OnErrorSubscription,
   OnResultSubscription,
   QueryPromise,
@@ -97,6 +98,7 @@ export class QueryManager {
   addSubscription<Data, Variables>(
     queryRef: OperationRef<Data, Variables>,
     onResultCallback: OnResultSubscription<Data, Variables>,
+    onCompleteCallback: OnCompleteSubscription,
     onErrorCallback?: OnErrorSubscription,
     initialCache?: OpResult<Data>
   ): () => void {
@@ -111,13 +113,15 @@ export class QueryManager {
     >;
     const subscription = {
       userCallback: onResultCallback,
+      onCompleteCallback,
       errCallback: onErrorCallback
     };
     const unsubscribe = (): void => {
       const trackedQuery = this._queries.get(key)!;
       trackedQuery.subscriptions = trackedQuery.subscriptions.filter(
         sub => sub !== subscription
       );
+      onCompleteCallback();
     };
     if (initialCache && trackedQuery.currentCache !== initialCache) {
       logDebug('Initial cache found. Comparing dates.');

packages/data-connect/test/unit/queries.test.ts

@@ -21,13 +21,14 @@ import * as chai from 'chai';
 import chaiAsPromised from 'chai-as-promised';
 import * as sinon from 'sinon';
 
-import { DataConnectOptions } from '../../src';
+import { DataConnectOptions, QueryRef, queryRef, subscribe } from '../../src';
 import {
   AuthTokenListener,
   AuthTokenProvider
 } from '../../src/core/FirebaseAuthProvider';
 import { initializeFetch } from '../../src/network/fetch';
 import { RESTTransport } from '../../src/network/transport/rest';
+import { initDatabase } from '../util';
 chai.use(chaiAsPromised);
 const options: DataConnectOptions = {
   connector: 'c',
@@ -61,10 +62,102 @@ const fakeFetchImpl = sinon.stub().returns(
     status: 401
   } as Response)
 );
+interface PostVariables {
+  testId: string;
+}
+const TEST_ID = crypto.randomUUID();
+interface PostListResponse {
+  posts: Post[];
+}
+interface Post {
+  id: string;
+  description: string;
+}
+function getPostsRef(): QueryRef<PostListResponse, PostVariables> {
+  const dc = initDatabase();
+  return queryRef<PostListResponse, PostVariables>(dc, 'ListPosts', {
+    testId: TEST_ID
+  });
+}
 describe('Queries', () => {
   afterEach(() => {
     fakeFetchImpl.resetHistory();
   });
+  it('should call onComplete callback after subscribe is called', async () => {
+    const taskListQuery = getPostsRef();
+    const onCompleteUserStub = sinon.stub();
+    const unsubscribe = subscribe(taskListQuery, {
+      onNext: () => {},
+      onComplete: onCompleteUserStub
+    });
+    expect(onCompleteUserStub).to.not.have.been.called;
+    unsubscribe();
+    expect(onCompleteUserStub).to.have.been.calledOnce;
+  });
+  it('should call onErr callback after a 401 occurs', async () => {
+    const json = {};
+    const throwErrorFakeImpl = sinon.stub().returns(
+      Promise.resolve({
+        json: () => {
+          return Promise.resolve(json);
+        },
+        status: 401
+      } as Response)
+    );
+    initializeFetch(throwErrorFakeImpl);
+    const taskListQuery = getPostsRef();
+    const onErrStub = sinon.stub();
+    let unsubscribeFn: (() => void) | null = null;
+    const promise = new Promise((resolve, reject) => {
+      unsubscribeFn = subscribe(taskListQuery, {
+        onNext: () => {
+          resolve(null);
+        },
+        onComplete: () => {},
+        onErr: err => {
+          onErrStub();
+          reject(err);
+        }
+      });
+    });
+    expect(onErrStub).not.to.have.been.called;
+    await expect(promise).to.have.eventually.been.rejected;
+    expect(onErrStub).to.have.been.calledOnce;
+    unsubscribeFn!();
+  });
+  it('should call onErr callback after a graphql error occurs', async () => {
+    const json = {
+      errors: [{ something: 'abc' }]
+    };
+    const throwErrorFakeImpl = sinon.stub().returns(
+      Promise.resolve({
+        json: () => {
+          return Promise.resolve(json);
+        },
+        status: 200
+      } as Response)
+    );
+    initializeFetch(throwErrorFakeImpl);
+    const taskListQuery = getPostsRef();
+    const onErrStub = sinon.stub();
+    let unsubscribeFn: (() => void) | null = null;
+    const promise = new Promise((resolve, reject) => {
+      unsubscribeFn = subscribe(taskListQuery, {
+        onNext: () => {
+          resolve(null);
+        },
+        onComplete: () => {},
+        onErr: err => {
+          onErrStub();
+          reject(err);
+        }
+      });
+    });
+    expect(onErrStub).not.to.have.been.called;
+    await expect(promise).to.have.eventually.been.rejected;
+    expect(onErrStub).to.have.been.calledOnce;
+    unsubscribeFn!();
+  });
   it('[QUERY] should retry auth whenever the fetcher returns with unauthorized', async () => {
     initializeFetch(fakeFetchImpl);
     const authProvider = new FakeAuthProvider();