Fixes for Bundles@exp (#4355)

Author: schmidt-sebastian

packages/firestore/exp/index.ts

@@ -37,6 +37,12 @@ export {
   namedQuery
 } from '../src/exp/database';
 
+export {
+  LoadBundleTask,
+  LoadBundleTaskProgress,
+  TaskState
+} from '../src/exp/bundle';
+
 export { Settings, PersistenceSettings } from '../src/exp/settings';
 
 export {

packages/firestore/export.ts

@@ -36,4 +36,4 @@ export { FieldPath } from './src/api/field_path';
 export { FieldValue } from './src/api/field_value';
 export { Timestamp } from './src/api/timestamp';
 export { FirebaseFirestore as ExpFirebaseFirestore } from './src/exp/database';
-export { loadBundle, namedQuery } from './src/api/bundle';
+export { loadBundle, namedQuery } from './src/exp/database';

packages/firestore/index.bundle.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { Firestore, loadBundle, namedQuery } from './export';
+import { Firestore, loadBundle, namedQuery, Query } from './export';
 
 /**
  * Prototype patches bundle loading to Firestore.
@@ -25,13 +25,20 @@ export function registerBundle(instance: typeof Firestore): void {
     this: Firestore,
     data: ArrayBuffer | ReadableStream<Uint8Array> | string
   ) {
-    return loadBundle(this, data);
+    return loadBundle(this._delegate, data);
   };
-  instance.prototype.namedQuery = function (
+  instance.prototype.namedQuery = async function (
     this: Firestore,
     queryName: string
   ) {
-    return namedQuery(this, queryName);
+    const expQuery = await namedQuery(this._delegate, queryName);
+    if (!expQuery) return null;
+    return new Query(
+      this,
+      // We can pass the exp-query here directly since named queries don't have UserDataConverters
+      // Otherwise we would have to create a new ExpQuery and pass the old UserDataConverter
+      expQuery
+    );
   };
 }
 

packages/firestore/src/api/database.ts

@@ -122,7 +122,6 @@ import {
 import { setLogLevel as setClientLogLevel } from '../util/log';
 
 import { Blob } from './blob';
-import { LoadBundleTask } from './bundle';
 import { Compat } from './compat';
 import {
   CompleteFn,
@@ -133,6 +132,7 @@ import {
 } from './observer';
 import { UntypedFirestoreDataConverter } from './user_data_reader';
 import { AbstractUserDataWriter } from './user_data_writer';
+import { LoadBundleTask } from '../exp/bundle';
 
 /**
  * A persistence provider for either memory-only or IndexedDB persistence.

packages/firestore/src/api/bundle.ts renamed to packages/firestore/src/exp/bundle.ts

@@ -15,26 +15,30 @@
  * limitations under the License.
  */
 
-import {
-  LoadBundleTask as ApiLoadBundleTask,
-  LoadBundleTaskProgress
-} from '@firebase/firestore-types';
-
-import { ensureFirestoreConfigured } from '../../src/exp/database';
-import { Query as ExpQuery } from '../../src/exp/reference';
-import {
-  firestoreClientGetNamedQuery,
-  firestoreClientLoadBundle
-} from '../core/firestore_client';
 import { debugAssert } from '../util/assert';
 import { FirestoreError } from '../util/error';
 import { Deferred } from '../util/promise';
+import { PartialObserver } from '../api/observer';
+
+// I moved this filed to match the current pattern where api relies on exp.
+// I also added all interfaces here, since we want this file to be the source
+// of truth for the generated API (if you run yarn:build it should generate
+// a new API report with these changes)
+
+// Add comments
+export type TaskState = 'Error' | 'Running' | 'Success';
+
+// Add comments
+export interface LoadBundleTaskProgress {
+  documentsLoaded: number;
+  totalDocuments: number;
+  bytesLoaded: number;
+  totalBytes: number;
+  taskState: TaskState;
+}
 
-import { Query, Firestore } from './database';
-import { PartialObserver } from './observer';
-
-export class LoadBundleTask
-  implements ApiLoadBundleTask, PromiseLike<LoadBundleTaskProgress> {
+// Add comments
+export class LoadBundleTask implements PromiseLike<LoadBundleTaskProgress> {
   private _progressObserver: PartialObserver<LoadBundleTaskProgress> = {};
   private _taskCompletionResolver = new Deferred<LoadBundleTaskProgress>();
 
@@ -46,6 +50,7 @@ export class LoadBundleTask
     documentsLoaded: 0
   };
 
+  // Add comments
   onProgress(
     next?: (progress: LoadBundleTaskProgress) => unknown,
     error?: (err: Error) => unknown,
@@ -58,12 +63,14 @@ export class LoadBundleTask
     };
   }
 
+  // Add comments
   catch<R>(
     onRejected: (a: Error) => R | PromiseLike<R>
   ): Promise<R | LoadBundleTaskProgress> {
     return this._taskCompletionResolver.promise.catch(onRejected);
   }
 
+  // Add comments
   then<T, R>(
     onFulfilled?: (a: LoadBundleTaskProgress) => T | PromiseLike<T>,
     onRejected?: (a: Error) => R | PromiseLike<R>
@@ -122,30 +129,3 @@ export class LoadBundleTask
     }
   }
 }
-
-export function loadBundle(
-  db: Firestore,
-  bundleData: ArrayBuffer | ReadableStream<Uint8Array> | string
-): LoadBundleTask {
-  const resultTask = new LoadBundleTask();
-  firestoreClientLoadBundle(
-    ensureFirestoreConfigured(db._delegate),
-    db._databaseId,
-    bundleData,
-    resultTask
-  );
-  return resultTask;
-}
-
-export function namedQuery(db: Firestore, name: string): Promise<Query | null> {
-  return firestoreClientGetNamedQuery(
-    ensureFirestoreConfigured(db._delegate),
-    name
-  ).then(namedQuery => {
-    if (!namedQuery) {
-      return null;
-    }
-
-    return new Query(db, new ExpQuery(db._delegate, null, namedQuery.query));
-  });
-}