Make newConnection() synchronous

packages/firestore/exp/src/api/transaction.ts

@@ -68,15 +68,14 @@ export function runTransaction<T>(
   updateFunction: (transaction: firestore.Transaction) => Promise<T>
 ): Promise<T> {
   const firestoreClient = cast(firestore, Firestore);
-  return getDatastore(firestoreClient).then(async datastore => {
-    const deferred = new Deferred<T>();
-    new TransactionRunner<T>(
-      new AsyncQueue(),
-      datastore,
-      internalTransaction =>
-        updateFunction(new Transaction(firestoreClient, internalTransaction)),
-      deferred
-    ).run();
-    return deferred.promise;
-  });
+  const datastore = getDatastore(firestoreClient);
+  const deferred = new Deferred<T>();
+  new TransactionRunner<T>(
+    new AsyncQueue(),
+    datastore,
+    internalTransaction =>
+      updateFunction(new Transaction(firestoreClient, internalTransaction)),
+    deferred
+  ).run();
+  return deferred.promise;
 }

packages/firestore/lite/src/api/components.ts

@@ -37,14 +37,14 @@ export const DEFAULT_SSL = true;
  * An instance map that ensures only one Datastore exists per Firestore
  * instance.
  */
-const datastoreInstances = new Map<Firestore, Promise<Datastore>>();
+const datastoreInstances = new Map<Firestore, Datastore>();
 
 /**
  * Returns an initialized and started Datastore for the given Firestore
  * instance. Callers must invoke removeDatastore() when the Firestore
  * instance is terminated.
  */
-export function getDatastore(firestore: Firestore): Promise<Datastore> {
+export function getDatastore(firestore: Firestore): Datastore {
   if (firestore._terminated) {
     throw new FirestoreError(
       Code.FAILED_PRECONDITION,
@@ -61,13 +61,16 @@ export function getDatastore(firestore: Firestore): Promise<Datastore> {
       settings.ssl ?? DEFAULT_SSL,
       /* forceLongPolling= */ false
     );
-    const datastorePromise = newConnection(databaseInfo).then(connection => {
-      const serializer = newSerializer(databaseInfo.databaseId);
-      const datastore = newDatastore(firestore._credentials, serializer);
-      datastore.start(connection);
-      return datastore;
-    });
-    datastoreInstances.set(firestore, datastorePromise);
+
+    const connection = newConnection(databaseInfo);
+    const serializer = newSerializer(databaseInfo.databaseId);
+    const datastore = newDatastore(
+      firestore._credentials,
+      connection,
+      serializer
+    );
+
+    datastoreInstances.set(firestore, datastore);
   }
   return datastoreInstances.get(firestore)!;
 }
@@ -76,11 +79,11 @@ export function getDatastore(firestore: Firestore): Promise<Datastore> {
  * Removes all components associated with the provided instance. Must be called
  * when the Firestore instance is terminated.
  */
-export async function removeComponents(firestore: Firestore): Promise<void> {
-  const datastorePromise = await datastoreInstances.get(firestore);
-  if (datastorePromise) {
+export function removeComponents(firestore: Firestore): void {
+  const datastore = datastoreInstances.get(firestore);
+  if (datastore) {
     logDebug(LOG_TAG, 'Removing Datastore');
     datastoreInstances.delete(firestore);
-    return (await datastorePromise).termiate();
+    datastore.terminate();
   }
 }

packages/firestore/lite/src/api/database.ts

@@ -110,7 +110,8 @@ export class Firestore
    * Only ever called once.
    */
   protected _terminate(): Promise<void> {
-    return removeComponents(this);
+    removeComponents(this);
+    return Promise.resolve();
   }
 
   // TODO(firestoreexp): `deleteApp()` should call the delete method above,

packages/firestore/lite/src/api/reference.ts

@@ -547,8 +547,9 @@ export function getDoc<T>(
   reference: firestore.DocumentReference<T>
 ): Promise<firestore.DocumentSnapshot<T>> {
   const ref = cast<DocumentReference<T>>(reference, DocumentReference);
-  return getDatastore(ref.firestore).then(async datastore => {
-    const result = await invokeBatchGetDocumentsRpc(datastore, [ref._key]);
+
+  const datastore = getDatastore(ref.firestore);
+  return invokeBatchGetDocumentsRpc(datastore, [ref._key]).then(result => {
     hardAssert(result.length === 1, 'Expected a single document result');
     const maybeDocument = result[0];
     return new DocumentSnapshot<T>(
@@ -563,21 +564,22 @@ export function getDoc<T>(
 export function getDocs<T>(
   query: firestore.Query<T>
 ): Promise<firestore.QuerySnapshot<T>> {
-  const internalQuery = cast<Query<T>>(query, Query);
-  validateHasExplicitOrderByForLimitToLast(internalQuery._query);
-  return getDatastore(internalQuery.firestore).then(async datastore => {
-    const result = await invokeRunQueryRpc(datastore, internalQuery._query);
+  const queryImpl = cast<Query<T>>(query, Query);
+  validateHasExplicitOrderByForLimitToLast(queryImpl._query);
+
+  const datastore = getDatastore(queryImpl.firestore);
+  return invokeRunQueryRpc(datastore, queryImpl._query).then(result => {
     const docs = result.map(
       doc =>
         new QueryDocumentSnapshot<T>(
-          internalQuery.firestore,
+          queryImpl.firestore,
           doc.key,
           doc,
-          internalQuery.converter
+          queryImpl.converter
         )
     );
 
-    if (internalQuery._query.hasLimitToLast()) {
+    if (queryImpl._query.hasLimitToLast()) {
       // Limit to last queries reverse the orderBy constraint that was
       // specified by the user. As such, we need to reverse the order of the
       // results to return the documents in the expected order.
@@ -619,11 +621,10 @@ export function setDoc<T>(
     options
   );
 
-  return getDatastore(ref.firestore).then(datastore =>
-    invokeCommitRpc(
-      datastore,
-      parsed.toMutations(ref._key, Precondition.none())
-    )
+  const datastore = getDatastore(ref.firestore);
+  return invokeCommitRpc(
+    datastore,
+    parsed.toMutations(ref._key, Precondition.none())
   );
 }
 
@@ -668,23 +669,21 @@ export function updateDoc(
     );
   }
 
-  return getDatastore(ref.firestore).then(datastore =>
-    invokeCommitRpc(
-      datastore,
-      parsed.toMutations(ref._key, Precondition.exists(true))
-    )
+  const datastore = getDatastore(ref.firestore);
+  return invokeCommitRpc(
+    datastore,
+    parsed.toMutations(ref._key, Precondition.exists(true))
   );
 }
 
 export function deleteDoc(
   reference: firestore.DocumentReference
 ): Promise<void> {
   const ref = cast<DocumentReference<unknown>>(reference, DocumentReference);
-  return getDatastore(ref.firestore).then(datastore =>
-    invokeCommitRpc(datastore, [
-      new DeleteMutation(ref._key, Precondition.none())
-    ])
-  );
+  const datastore = getDatastore(ref.firestore);
+  return invokeCommitRpc(datastore, [
+    new DeleteMutation(ref._key, Precondition.none())
+  ]);
 }
 
 export function addDoc<T>(
@@ -706,14 +705,11 @@ export function addDoc<T>(
     {}
   );
 
-  return getDatastore(collRef.firestore)
-    .then(datastore =>
-      invokeCommitRpc(
-        datastore,
-        parsed.toMutations(docRef._key, Precondition.exists(false))
-      )
-    )
-    .then(() => docRef);
+  const datastore = getDatastore(collRef.firestore);
+  return invokeCommitRpc(
+    datastore,
+    parsed.toMutations(docRef._key, Precondition.exists(false))
+  ).then(() => docRef);
 }
 
 export function refEqual<T>(

packages/firestore/lite/src/api/transaction.ts

@@ -178,15 +178,14 @@ export function runTransaction<T>(
   updateFunction: (transaction: firestore.Transaction) => Promise<T>
 ): Promise<T> {
   const firestoreClient = cast(firestore, Firestore);
-  return getDatastore(firestoreClient).then(async datastore => {
-    const deferred = new Deferred<T>();
-    new TransactionRunner<T>(
-      new AsyncQueue(),
-      datastore,
-      internalTransaction =>
-        updateFunction(new Transaction(firestoreClient, internalTransaction)),
-      deferred
-    ).run();
-    return deferred.promise;
-  });
+  const datastore = getDatastore(firestoreClient);
+  const deferred = new Deferred<T>();
+  new TransactionRunner<T>(
+    new AsyncQueue(),
+    datastore,
+    internalTransaction =>
+      updateFunction(new Transaction(firestoreClient, internalTransaction)),
+    deferred
+  ).run();
+  return deferred.promise;
 }

packages/firestore/lite/src/api/write_batch.ts

@@ -181,9 +181,8 @@ export function writeBatch(
   firestore: firestore.FirebaseFirestore
 ): firestore.WriteBatch {
   const firestoreImpl = cast(firestore, Firestore);
+  const datastore = getDatastore(firestoreImpl);
   return new WriteBatch(firestoreImpl, writes =>
-    getDatastore(firestoreImpl).then(datastore =>
-      invokeCommitRpc(datastore, writes)
-    )
+    invokeCommitRpc(datastore, writes)
   );
 }

packages/firestore/src/core/component_provider.ts

@@ -61,7 +61,6 @@ import { newConnection, newConnectivityMonitor } from '../platform/connection';
 import { newSerializer } from '../platform/serializer';
 import { getDocument, getWindow } from '../platform/dom';
 import { CredentialsProvider } from '../api/credentials';
-import { Connection } from '../remote/connection';
 
 const MEMORY_ONLY_PERSISTENCE_ERROR_MESSAGE =
   'You are using the memory-only build of Firestore. Persistence support is ' +
@@ -329,9 +328,6 @@ export class OnlineComponentProvider {
     this.localStore = offlineComponentProvider.localStore;
     this.sharedClientState = offlineComponentProvider.sharedClientState;
     this.datastore = this.createDatastore(cfg);
-    const connection = await this.loadConnection(cfg);
-    this.datastore.start(connection);
-
     this.remoteStore = this.createRemoteStore(cfg);
     this.syncEngine = this.createSyncEngine(cfg);
     this.eventManager = this.createEventManager(cfg);
@@ -348,17 +344,14 @@ export class OnlineComponentProvider {
     await this.remoteStore.applyPrimaryState(this.syncEngine.isPrimaryClient);
   }
 
-  protected loadConnection(cfg: ComponentConfiguration): Promise<Connection> {
-    return newConnection(cfg.databaseInfo);
-  }
-
   createEventManager(cfg: ComponentConfiguration): EventManager {
     return new EventManager(this.syncEngine);
   }
 
   createDatastore(cfg: ComponentConfiguration): Datastore {
     const serializer = newSerializer(cfg.databaseInfo.databaseId);
-    return newDatastore(cfg.credentials, serializer);
+    const connection = newConnection(cfg.databaseInfo);
+    return newDatastore(cfg.credentials, connection, serializer);
   }
 
   createRemoteStore(cfg: ComponentConfiguration): RemoteStore {

packages/firestore/src/platform/browser/connection.ts

@@ -23,8 +23,8 @@ import { BrowserConnectivityMonitor } from './connectivity_monitor';
 import { NoopConnectivityMonitor } from '../../remote/connectivity_monitor_noop';
 
 /** Initializes the WebChannelConnection for the browser. */
-export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
-  return Promise.resolve(new WebChannelConnection(databaseInfo));
+export function newConnection(databaseInfo: DatabaseInfo): Connection {
+  return new WebChannelConnection(databaseInfo);
 }
 
 /** Return the Platform-specific connectivity monitor. */

packages/firestore/src/platform/browser_lite/connection.ts

@@ -22,6 +22,6 @@ import { FetchConnection } from './fetch_connection';
 export { newConnectivityMonitor } from '../browser/connection';
 
 /** Initializes the HTTP connection for the REST API. */
-export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
-  return Promise.resolve(new FetchConnection(databaseInfo, fetch.bind(null)));
+export function newConnection(databaseInfo: DatabaseInfo): Connection {
+  return new FetchConnection(databaseInfo, fetch.bind(null));
 }

packages/firestore/src/platform/connection.ts

@@ -26,7 +26,6 @@ export function newConnectivityMonitor(): ConnectivityMonitor {
   return platform.newConnectivityMonitor();
 }
 
-// TODO(firestorexp): This doesn't need to return a Promise
-export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
+export function newConnection(databaseInfo: DatabaseInfo): Connection {
   return platform.newConnection(databaseInfo);
 }

packages/firestore/src/platform/node/connection.ts

@@ -23,9 +23,9 @@ import { Connection } from '../../remote/connection';
 import { NoopConnectivityMonitor } from '../../remote/connectivity_monitor_noop';
 
 /** Loads the GRPC stack */
-export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
+export function newConnection(databaseInfo: DatabaseInfo): Connection {
   const protos = loadProtos();
-  return Promise.resolve(new GrpcConnection(protos, databaseInfo));
+  return new GrpcConnection(protos, databaseInfo);
 }
 
 /** Return the Platform-specific connectivity monitor. */

packages/firestore/src/platform/node_lite/connection.ts

@@ -24,9 +24,9 @@ import { Connection } from '../../remote/connection';
 export { newConnectivityMonitor } from '../browser/connection';
 
 /** Initializes the HTTP connection for the REST API. */
-export function newConnection(databaseInfo: DatabaseInfo): Promise<Connection> {
-  // node-fetch is meant to be API compatible with `fetch`, but its type don't
+export function newConnection(databaseInfo: DatabaseInfo): Connection {
+  // node-fetch is meant to be API compatible with `fetch`, but its type doesn't
   // match 100%.
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  return Promise.resolve(new FetchConnection(databaseInfo, nodeFetch as any));
+  return new FetchConnection(databaseInfo, nodeFetch as any);
 }

packages/firestore/src/remote/datastore.ts

@@ -47,20 +47,19 @@ import { Query, queryToTarget } from '../core/query';
  * for the rest of the client SDK architecture to consume.
  */
 export abstract class Datastore {
-  abstract start(connection: Connection): void;
-  abstract termiate(): Promise<void>;
+  abstract terminate(): void;
 }
 
 /**
  * An implementation of Datastore that exposes additional state for internal
  * consumption.
  */
 class DatastoreImpl extends Datastore {
-  connection!: Connection;
   terminated = false;
 
   constructor(
     readonly credentials: CredentialsProvider,
+    readonly connection: Connection,
     readonly serializer: JsonProtoSerializer
   ) {
     super();
@@ -76,11 +75,6 @@ class DatastoreImpl extends Datastore {
     }
   }
 
-  start(connection: Connection): void {
-    debugAssert(!this.connection, 'Datastore.start() already called');
-    this.connection = connection;
-  }
-
   /** Gets an auth token and invokes the provided RPC. */
   invokeRPC<Req, Resp>(
     rpcName: string,
@@ -131,7 +125,7 @@ class DatastoreImpl extends Datastore {
       });
   }
 
-  async termiate(): Promise<void> {
+  terminate(): void {
     this.terminated = false;
   }
 }
@@ -140,9 +134,10 @@ class DatastoreImpl extends Datastore {
 // firestore-exp client.
 export function newDatastore(
   credentials: CredentialsProvider,
+  connection: Connection,
   serializer: JsonProtoSerializer
 ): Datastore {
-  return new DatastoreImpl(credentials, serializer);
+  return new DatastoreImpl(credentials, connection, serializer);
 }
 
 export async function invokeCommitRpc(