refactor(database): Add types to several classes and utility methods (#66)

* refactor(database): Several classes and utility methods

* WIP: fix tests with recent type changes

* refactor(database): Type casting format for TSX support

* refactor(database): Add 'noImplicitAny' support
  Adds support for the 'noImplicitAny' Typescript compiler option to the database implementation.

Author: jsayol

src/database.ts

@@ -25,7 +25,7 @@ import * as INTERNAL from './database/api/internal';
 import * as TEST_ACCESS from './database/api/test_access';
 import { isNodeSdk } from "./utils/environment";
 
-export function registerDatabase(instance) {
+export function registerDatabase(instance: FirebaseNamespace) {
   // Register the Database Service with the 'firebase' namespace.
   const namespace = instance.INTERNAL.registerService(
     'database',

src/database/api/DataSnapshot.ts

@@ -117,7 +117,7 @@ export class DataSnapshot {
     validateArgCount('DataSnapshot.getPriority', 0, 0, arguments.length);
 
     // typecast here because we never return deferred values or internal priorities (MAX_PRIORITY)
-    return /**@type {string|number|null} */ <string | number | null>(this.node_.getPriority().val());
+    return (this.node_.getPriority().val() as string | number | null);
   }
 
   /**
@@ -128,14 +128,14 @@ export class DataSnapshot {
    * @return {boolean} True if forEach was canceled by action returning true for
    * one of the child nodes.
    */
-  forEach(action: (d: DataSnapshot) => any): boolean {
+  forEach(action: (d: DataSnapshot) => void): boolean {
     validateArgCount('DataSnapshot.forEach', 1, 1, arguments.length);
     validateCallback('DataSnapshot.forEach', 1, action, false);
 
     if (this.node_.isLeafNode())
       return false;
 
-    const childrenNode = /**@type {ChildrenNode} */ <ChildrenNode>(this.node_);
+    const childrenNode = (this.node_ as ChildrenNode);
     // Sanitize the return value to a boolean. ChildrenNode.forEachChild has a weird return type...
     return !!childrenNode.forEachChild(this.index_, (key, node) => {
       return action(new DataSnapshot(node, this.ref_.child(key), PRIORITY_INDEX));

src/database/api/Database.ts

@@ -1,59 +1,56 @@
 /**
-* Copyright 2017 Google Inc.
-*
-* Licensed under the Apache License, Version 2.0 (the "License");
-* you may not use this file except in compliance with the License.
-* You may obtain a copy of the License at
-*
-*   http://www.apache.org/licenses/LICENSE-2.0
-*
-* Unless required by applicable law or agreed to in writing, software
-* distributed under the License is distributed on an "AS IS" BASIS,
-* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-* See the License for the specific language governing permissions and
-* limitations under the License.
-*/
-
-import { fatal } from "../core/util/util";
-import { parseRepoInfo } from "../core/util/libs/parser";
-import { Path } from "../core/util/Path";
-import { PromiseImpl } from "../../utils/promise";
-import { Reference } from "./Reference";
-import { Repo } from "../core/Repo";
-import { RepoManager } from "../core/RepoManager";
-import { validateArgCount } from "../../utils/validation";
-import { FirebaseApp } from "../../app/firebase_app";
-import { validateUrl } from "../core/util/validation";
+ * Copyright 2017 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { fatal } from '../core/util/util';
+import { parseRepoInfo } from '../core/util/libs/parser';
+import { Path } from '../core/util/Path';
+import { PromiseImpl } from '../../utils/promise';
+import { Reference } from './Reference';
+import { Repo } from '../core/Repo';
+import { RepoManager } from '../core/RepoManager';
+import { validateArgCount } from '../../utils/validation';
+import { validateUrl } from '../core/util/validation';
+import { FirebaseApp, FirebaseService } from '../../app/firebase_app';
+import { RepoInfo } from '../core/RepoInfo';
 
 /**
  * Class representing a firebase database.
- * @implements {firebase.Service}
+ * @implements {FirebaseService}
  */
-export class Database {
-  repo_: Repo;
-  root_: Reference;
-  INTERNAL;
-  
-  static ServerValue = {
+export class Database implements FirebaseService {
+  INTERNAL: DatabaseInternals;
+  private root_: Reference;
+
+  static readonly ServerValue = {
     'TIMESTAMP': {
-      '.sv' : 'timestamp'
+      '.sv': 'timestamp'
     }
-  }
+  };
 
   /**
    * The constructor should not be called by users of our public API.
-   * @param {!Repo} repo
+   * @param {!Repo} repo_
    */
-  constructor(repo) {
-    if (!(repo instanceof Repo)) {
-      fatal("Don't call new Database() directly - please use firebase.database().");
+  constructor(private repo_: Repo) {
+    if (!(repo_ instanceof Repo)) {
+      fatal('Don\'t call new Database() directly - please use firebase.database().');
     }
 
-    /** @type {Repo} */
-    this.repo_ = repo;
-
-    /** @type {Firebase} */
-    this.root_ = new Reference(repo, Path.Empty);
+    /** @type {Reference} */
+    this.root_ = new Reference(repo_, Path.Empty);
 
     this.INTERNAL = new DatabaseInternals(this);
   }
@@ -65,7 +62,7 @@ export class Database {
   /**
    * Returns a reference to the root or the path specified in opt_pathString.
    * @param {string=} pathString
-   * @return {!Firebase} Firebase reference.
+   * @return {!Reference} Firebase reference.
    */
   ref(pathString?: string): Reference {
     this.checkDeleted_('ref');
@@ -79,20 +76,21 @@ export class Database {
    * We throw a exception if the url is not in the same domain as the
    * current repo.
    * @param {string} url
-   * @return {!Firebase} Firebase reference.
+   * @return {!Reference} Firebase reference.
    */
-  refFromURL(url) {
+  refFromURL(url: string): Reference {
     /** @const {string} */
-    var apiName = 'database.refFromURL';
+    const apiName = 'database.refFromURL';
     this.checkDeleted_(apiName);
     validateArgCount(apiName, 1, 1, arguments.length);
-    var parsedURL = parseRepoInfo(url);
+    const parsedURL = parseRepoInfo(url);
     validateUrl(apiName, 1, parsedURL);
 
-    var repoInfo = parsedURL.repoInfo;
-    if (repoInfo.host !== this.repo_.repoInfo_.host) {
-      fatal(apiName + ": Host name does not match the current database: " +
-                         "(found " + repoInfo.host + " but expected " + this.repo_.repoInfo_.host + ")");
+    const repoInfo = parsedURL.repoInfo;
+    if (repoInfo.host !== ((this.repo_ as any).repoInfo_ as RepoInfo).host) {
+      fatal(apiName + ': Host name does not match the current database: ' +
+        '(found ' + repoInfo.host + ' but expected ' +
+        ((this.repo_ as any).repoInfo_ as RepoInfo).host + ')');
     }
 
     return this.ref(parsedURL.path.toString());
@@ -101,9 +99,9 @@ export class Database {
   /**
    * @param {string} apiName
    */
-  private checkDeleted_(apiName) {
+  private checkDeleted_(apiName: string) {
     if (this.repo_ === null) {
-      fatal("Cannot call " + apiName + " on a deleted database.");
+      fatal('Cannot call ' + apiName + ' on a deleted database.');
     }
   }
 
@@ -114,36 +112,28 @@ export class Database {
     this.repo_.interrupt();
   }
 
-  goOnline () {
+  goOnline() {
     validateArgCount('database.goOnline', 0, 0, arguments.length);
     this.checkDeleted_('goOnline');
     this.repo_.resume();
   }
-};
-
-Object.defineProperty(Repo.prototype, 'database', {		
-  get() {		
-    return this.__database || (this.__database = new Database(this));		
-  }		
-});
+}
 
-class DatabaseInternals {
-  database
+export class DatabaseInternals {
   /** @param {!Database} database */
-  constructor(database) {
-    this.database = database;
+  constructor(public database: Database) {
   }
 
-  /** @return {firebase.Promise<void>} */
-  delete() {
-    this.database.checkDeleted_('delete');
-    RepoManager.getInstance().deleteRepo(/** @type {!Repo} */ (this.database.repo_));
+  /** @return {Promise<void>} */
+  delete(): Promise<void> {
+    (this.database as any).checkDeleted_('delete');
+    RepoManager.getInstance().deleteRepo((this.database as any).repo_ as Repo);
 
-    this.database.repo_ = null;
-    this.database.root_ = null;
+    (this.database as any).repo_ = null;
+    (this.database as any).root_ = null;
     this.database.INTERNAL = null;
     this.database = null;
     return PromiseImpl.resolve();
   }
-};
+}
 

src/database/api/Query.ts

@@ -29,7 +29,7 @@ import {
   validateKey,
 } from '../core/util/validation';
 import { errorPrefix, validateArgCount, validateCallback, validateContextObject } from '../../utils/validation';
-import { ValueEventRegistration, ChildEventRegistration } from '../core/view/EventRegistration';
+import { ValueEventRegistration, ChildEventRegistration, EventRegistration } from '../core/view/EventRegistration';
 import { Deferred, attachDummyErrorHandler } from '../../utils/promise';
 import { Repo } from '../core/Repo';
 import { QueryParams } from '../core/view/QueryParams';
@@ -153,7 +153,7 @@ export class Query {
     // This is a slight hack. We cannot goog.require('fb.api.Firebase'), since Firebase requires fb.api.Query.
     // However, we will always export 'Firebase' to the global namespace, so it's guaranteed to exist by the time this
     // method gets called.
-    return <Reference>(new Query.__referenceConstructor(this.repo, this.path));
+    return (new Query.__referenceConstructor(this.repo, this.path) as Reference);
   }
 
   /**
@@ -174,7 +174,7 @@ export class Query {
     if (eventType === 'value') {
       this.onValueEvent(callback, ret.cancel, ret.context);
     } else {
-      const callbacks = {};
+      const callbacks: { [k: string]: typeof callback } = {};
       callbacks[eventType] = callback;
       this.onChildEvent(callbacks, ret.cancel, ret.context);
     }
@@ -187,7 +187,7 @@ export class Query {
    * @param {?Object} context
    * @protected
    */
-  onValueEvent(callback: (a: DataSnapshot) => any, cancelCallback: ((a: Error) => any) | null, context: Object | null) {
+  protected onValueEvent(callback: (a: DataSnapshot) => void, cancelCallback: ((a: Error) => void) | null, context: Object | null) {
     const container = new ValueEventRegistration(callback, cancelCallback || null, context || null);
     this.repo.addEventCallbackForQuery(this, container);
   }
@@ -196,6 +196,7 @@ export class Query {
    * @param {!Object.<string, !function(!DataSnapshot, ?string)>} callbacks
    * @param {?function(Error)} cancelCallback
    * @param {?Object} context
+   * @protected
    */
   onChildEvent(callbacks: { [k: string]: SnapshotCallback },
                cancelCallback: ((a: Error) => any) | null, context: Object | null) {
@@ -214,10 +215,10 @@ export class Query {
     validateCallback('Query.off', 2, callback, true);
     validateContextObject('Query.off', 3, context, true);
 
-    let container = null;
-    let callbacks = null;
+    let container: EventRegistration | null = null;
+    let callbacks: { [k: string]: typeof callback } | null = null;
     if (eventType === 'value') {
-      const valueCallback = /** @type {function(!DataSnapshot)} */ (callback) || null;
+      const valueCallback = callback || null;
       container = new ValueEventRegistration(valueCallback, null, context || null);
     } else if (eventType) {
       if (callback) {
@@ -239,7 +240,8 @@ export class Query {
    */
   once(eventType: string, 
        userCallback?: SnapshotCallback,
-       cancelOrContext?, context?: Object): Promise<DataSnapshot> {
+       cancelOrContext?: ((a: Error) => void) | Object,
+       context?: Object): Promise<DataSnapshot> {
     validateArgCount('Query.once', 1, 4, arguments.length);
     validateEventType('Query.once', 1, eventType, false);
     validateCallback('Query.once', 2, userCallback, true);
@@ -254,7 +256,7 @@ export class Query {
     const deferred = new Deferred();
     attachDummyErrorHandler(deferred.promise);
 
-    const onceCallback = (snapshot) => {
+    const onceCallback = (snapshot: DataSnapshot) => {
       // NOTE: Even though we unsubscribe, we may get called multiple times if a single action (e.g. set() with JSON)
       // triggers multiple events (e.g. child_added or child_changed).
       if (firstCall) {
@@ -508,11 +510,11 @@ export class Query {
    * @return {{cancel: ?function(Error), context: ?Object}}
    * @private
    */
-  private static getCancelAndContextArgs_(fnName: string, cancelOrContext?: ((a: Error) => any) | Object,
-                                   context?: Object): { cancel: ((a: Error) => any) | null, context: Object | null } {
-    const ret = {cancel: null, context: null};
+  private static getCancelAndContextArgs_(fnName: string, cancelOrContext?: ((a: Error) => void) | Object,
+                                          context?: Object): { cancel: ((a: Error) => void) | null, context: Object | null } {
+    const ret: { cancel: ((a: Error) => void) | null, context: Object | null } = {cancel: null, context: null};
     if (cancelOrContext && context) {
-      ret.cancel = /** @type {function(Error)} */ (cancelOrContext);
+      ret.cancel = (cancelOrContext as (a: Error) => void);
       validateCallback(fnName, 3, ret.cancel, true);
 
       ret.context = context;