Remove return Promise.resolve() statements from the codebase (#422)

* Remove `return Promise.resolve()` statements from the codebase.

* Fix compilation errors.

* Fix more compiler errors.

* Use `tslib` module and `importHelpers` Typescript compiler option.

It reduces the size of TypeScript packages (namely `firebase-database`,
`firebase-firestore`, `firebase-messaging` and the combined `firebase`)
be reusing TypeScript helper methods (e.g. `__awaiter`) which were
included in every single file relying on features not available
in ES5 (emitted during transpilation process).

* [AUTOMATED]: Prettier Code Styling

* Regenerate top-level yarn.lock

* Add `tslib` as a dependency to app, polyfill, storage, template and util packages.

Since we set `importHelpers` compiler option to `true` in the base config,
we need to add `tslib` to all packages. `*-types` packages are omitted
since they do not contain any executable code.

* Revert unnecessary change in comment formatting.

* Revert moving comments out of empty else block to make
the context more clear.

Addresses @mikelehen review
#422 (review)

Author: merlinnot

config/tsconfig.base.json

@@ -2,6 +2,7 @@
   "compileOnSave": false,
   "compilerOptions": {
     "declaration": true,
+    "importHelpers": true,
     "lib": [
       "es5",
       "es2015",

integration/messaging/test/utils/test-server.js

@@ -42,9 +42,9 @@ class MessagingTestServer {
     return `http://localhost:${PORT_NUMBER}`;
   }
 
-  start() {
+  async start() {
     if (this._server) {
-      return Promise.resolve();
+      return;
     }
 
     return new Promise((resolve, reject) => {
@@ -56,15 +56,11 @@ class MessagingTestServer {
 
   // Sometimes the server doesn't trigger the callback due to
   // currently open sockets. So call `closethis._server
-  stop() {
-    if (!this._server) {
-      return Promise.resolve();
+  async stop() {
+    if (this._server) {
+      this._server.close();
+      this._server = null;
     }
-
-    this._server.close();
-    this._server = null;
-
-    return Promise.resolve();
   }
 }
 module.exports = new MessagingTestServer();

packages/app/package.json

@@ -16,7 +16,8 @@
   "license": "Apache-2.0",
   "dependencies": {
     "@firebase/app-types": "0.1.1",
-    "@firebase/util": "0.1.9"
+    "@firebase/util": "0.1.9",
+    "tslib": "^1.9.0"
   },
   "devDependencies": {
     "@types/chai": "^4.0.4",

packages/database/package.json

@@ -21,7 +21,8 @@
   "dependencies": {
     "@firebase/database-types": "0.1.1",
     "@firebase/util": "0.1.9",
-    "faye-websocket": "0.11.1"
+    "faye-websocket": "0.11.1",
+    "tslib": "^1.9.0"
   },
   "devDependencies": {
     "@types/chai": "^4.0.4",

packages/database/src/api/Database.ts

@@ -132,14 +132,13 @@ export class DatabaseInternals {
   constructor(public database: Database) {}
 
   /** @return {Promise<void>} */
-  delete(): Promise<void> {
+  async delete(): Promise<void> {
     (this.database as any).checkDeleted_('delete');
     RepoManager.getInstance().deleteRepo((this.database as any).repo_ as Repo);
 
     (this.database as any).repo_ = null;
     (this.database as any).root_ = null;
     this.database.INTERNAL = null;
     this.database = null;
-    return Promise.resolve();
   }
 }

packages/firestore/package.json

@@ -24,7 +24,8 @@
   "dependencies": {
     "@firebase/firestore-types": "0.2.1",
     "@firebase/webchannel-wrapper": "0.2.6",
-    "grpc": "^1.7.1"
+    "grpc": "^1.7.1",
+    "tslib": "^1.9.0"
   },
   "peerDependencies": {
     "@firebase/app": "^0.1.0",

packages/firestore/src/api/database.ts

@@ -383,11 +383,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
   }
 
   INTERNAL = {
-    delete: (): Promise<void> => {
+    delete: async (): Promise<void> => {
       if (this._firestoreClient) {
         return this._firestoreClient.shutdown();
-      } else {
-        return Promise.resolve();
       }
     }
   };
@@ -685,13 +683,11 @@ export class WriteBatch implements firestore.WriteBatch {
     return this;
   }
 
-  commit(): Promise<void> {
+  async commit(): Promise<void> {
     this.verifyNotCommitted();
     this._committed = true;
     if (this._mutations.length > 0) {
       return this._firestore.ensureClientConfigured().write(this._mutations);
-    } else {
-      return Promise.resolve();
     }
   }
 

packages/firestore/src/core/event_manager.ts

@@ -87,7 +87,7 @@ export class EventManager {
     }
   }
 
-  unlisten(listener: QueryListener): Promise<void> {
+  async unlisten(listener: QueryListener): Promise<void> {
     const query = listener.query;
     let lastListen = false;
 
@@ -103,8 +103,6 @@ export class EventManager {
     if (lastListen) {
       this.queries.delete(query);
       return this.syncEngine.unlisten(query);
-    } else {
-      return Promise.resolve();
     }
   }
 

packages/firestore/src/core/firestore_client.ts

@@ -371,11 +371,7 @@ export class FirestoreClient {
   ): Promise<T> {
     // We have to wait for the async queue to be sure syncEngine is initialized.
     return this.asyncQueue
-      .enqueue(() => {
-        return Promise.resolve();
-      })
-      .then(() => {
-        return this.syncEngine.runTransaction(updateFunction);
-      });
+      .enqueue(async () => {})
+      .then(() => this.syncEngine.runTransaction(updateFunction));
   }
 }

packages/firestore/src/local/memory_persistence.ts

@@ -47,18 +47,16 @@ export class MemoryPersistence implements Persistence {
 
   private started = false;
 
-  start(): Promise<void> {
+  async start(): Promise<void> {
+    // No durable state to read on startup.
     assert(!this.started, 'MemoryPersistence double-started!');
     this.started = true;
-    // No durable state to read on startup.
-    return Promise.resolve();
   }
 
-  shutdown(): Promise<void> {
+  async shutdown(): Promise<void> {
     // No durable state to ensure is closed on shutdown.
     assert(this.started, 'MemoryPersistence shutdown without start!');
     this.started = false;
-    return Promise.resolve();
   }
 
   getMutationQueue(user: User): MutationQueue {

packages/firestore/src/remote/persistent_stream.ts

@@ -288,13 +288,12 @@ export abstract class PersistentStream<
   }
 
   /** Called by the idle timer when the stream should close due to inactivity. */
-  private handleIdleCloseTimer(): Promise<void> {
+  private async handleIdleCloseTimer(): Promise<void> {
     if (this.isOpen()) {
       // When timing out an idle stream there's no reason to force the stream into backoff when
       // it restarts so set the stream state to Initial instead of Error.
       return this.close(PersistentStreamState.Initial);
     }
-    return Promise.resolve();
   }
 
   /** Marks the stream as active again. */
@@ -319,7 +318,7 @@ export abstract class PersistentStream<
    * @param finalState the intended state of the stream after closing.
    * @param error the error the connection was closed with.
    */
-  private close(
+  private async close(
     finalState: PersistentStreamState,
     error?: FirestoreError
   ): Promise<void> {
@@ -361,8 +360,6 @@ export abstract class PersistentStream<
     // could trigger undesirable recovery logic, etc.).
     if (finalState !== PersistentStreamState.Stopped) {
       return listener.onClose(error);
-    } else {
-      return Promise.resolve();
     }
   }
 
@@ -403,16 +400,14 @@ export abstract class PersistentStream<
         this.startStream(token);
       },
       (error: Error) => {
-        this.queue.enqueue(() => {
+        this.queue.enqueue(async () => {
           if (this.state !== PersistentStreamState.Stopped) {
             // Stream can be stopped while waiting for authorization.
             const rpcError = new FirestoreError(
               Code.UNKNOWN,
               'Fetching auth token failed: ' + error.message
             );
             return this.handleStreamClose(rpcError);
-          } else {
-            return Promise.resolve();
           }
         });
       }
@@ -436,12 +431,10 @@ export abstract class PersistentStream<
       stream: Stream<SendType, ReceiveType>,
       fn: () => Promise<void>
     ) => {
-      this.queue.enqueue(() => {
+      this.queue.enqueue(async () => {
         // Only raise events if the stream instance has not changed
         if (this.stream === stream) {
           return fn();
-        } else {
-          return Promise.resolve();
         }
       });
     };
@@ -480,16 +473,15 @@ export abstract class PersistentStream<
     );
     this.state = PersistentStreamState.Backoff;
 
-    this.backoff.backoffAndRun(() => {
+    this.backoff.backoffAndRun(async () => {
       if (this.state === PersistentStreamState.Stopped) {
         // Stream can be stopped while waiting for backoff to complete.
-        return Promise.resolve();
+        return;
       }
 
       this.state = PersistentStreamState.Initial;
       this.start(listener);
       assert(this.isStarted(), 'PersistentStream should have started');
-      return Promise.resolve();
     });
   }