[query] - Update query.on() to follow web api and returns a Function instead of a promise

- Also remove ref from datasnapshot until we translate it into a web ref instead of native ref.

Author: KkevinLi

src/app/database/index.ts

@@ -17,7 +17,7 @@ export namespace database {
     val(): any;
   }
 
-  export class Query {
+  export class Query implements firebase.Query {
     protected path: string;
     private queryObject: firebase.Query;
     constructor(path: string) {
@@ -34,9 +34,8 @@ export namespace database {
     public on(eventType: string, callback: (a: DataSnapshot | null, b?: string) => any,
       cancelCallbackOrContext?: Object | null, context?: Object | null): (a: DataSnapshot | null, b?: string) => Function {
 
-      this.queryObject.on(eventType, callback).catch(error => {
-        console.log("firebase.database().on error: " + error);
-      });
+      this.queryObject.on(eventType, callback);
+
       return callback; // According to firebase doc we just return the callback given
     }
 

src/firebase.android.ts

@@ -1640,41 +1640,36 @@ class Query implements QueryBase {
     this.query = this.dbRef;
   }
 
-  on(eventType: string, callback: (a: any, b?: string) => any): Promise<any> {
+  on(eventType: string, callback: (a: any, b?: string) => any): Function {
     const onValueEvent = result => {
-        callback(result);
+      callback(result);
     };
 
-    return new Promise((resolve, reject) => {
-      try {
-        if (firebase.instance === null) {
-          reject("Run init() first!");
-          return;
-        }
-        const listener = this.createEventListener(eventType, onValueEvent);
+    try {
+      if (firebase.instance === null) {
+        throw new Error("Run init() first!");
+      }
+      const listener = this.createEventListener(eventType, onValueEvent);
 
-        if (eventType === "value") {
-          this.query.addValueEventListener(listener as com.google.firebase.database.ValueEventListener);
-        } else if (eventType === "child_added" || eventType === "child_changed" || eventType === "child_removed" || eventType === "child_moved") {
-          this.query.addChildEventListener(listener as com.google.firebase.database.ChildEventListener);
-        } else {
-          callback({
-            error: "Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'"
-          });
-          reject("Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'");
-          return;
-        }
-        // Add listener to our map which keeps track of eventType: child/value events
-        if (!Query.eventListenerMap.has(eventType)) {
-          Query.eventListenerMap.set(eventType, []);
-        }
-        Query.eventListenerMap.get(eventType).push(listener); // We need to keep track of the listeners to fully remove them when calling off
-        resolve();
-      } catch (ex) {
-        console.log("Error in firebase.on: " + ex);
-        reject(ex);
+      if (eventType === "value") {
+        this.query.addValueEventListener(listener as com.google.firebase.database.ValueEventListener);
+      } else if (eventType === "child_added" || eventType === "child_changed" || eventType === "child_removed" || eventType === "child_moved") {
+        this.query.addChildEventListener(listener as com.google.firebase.database.ChildEventListener);
+      } else {
+        callback({
+          error: "Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'"
+        });
       }
-    });
+      // Add listener to our map which keeps track of eventType: child/value events
+      if (!Query.eventListenerMap.has(eventType)) {
+        Query.eventListenerMap.set(eventType, []);
+      }
+      Query.eventListenerMap.get(eventType).push(listener); // We need to keep track of the listeners to fully remove them when calling off
+    } catch (ex) {
+      console.error("Error in firebase.on: " + ex);
+    } finally {
+      return callback;
+    }
   }
 
   once(eventType: string): Promise<DataSnapshot> {
@@ -2116,6 +2111,9 @@ firebase.transaction = (path: string, transactionUpdate: (currentState) => any,
   });
 };
 
+function nativeRefToWebRef(ref: com.google.firebase.database.DatabaseReference) {
+
+}
 // Converts Android DataSnapshot into Web Datasnapshot
 function nativeSnapshotToWebSnapshot(snapshot: com.google.firebase.database.DataSnapshot): DataSnapshot {
   function forEach(action: (datasnapshot: DataSnapshot) => any): boolean {
@@ -2131,7 +2129,7 @@ function nativeSnapshotToWebSnapshot(snapshot: com.google.firebase.database.Data
 
   return {
     key: snapshot.getKey(),
-    ref: snapshot.getRef(),
+    // ref: snapshot.getRef(), TODO: Convert native ref to webRef
     child: (path: string) => nativeSnapshotToWebSnapshot(snapshot.child(path)),
     exists: () => snapshot.exists(),
     forEach: (func: (datasnapshot) => any) => forEach(func),

src/firebase.d.ts

@@ -549,7 +549,7 @@ export interface OnDisconnect {
 
 // WebAPI Query
 export interface Query {
-  on(eventType: string, callback: (a: any, b?: string) => any): Promise<any>;
+  on(eventType: string, callback: (a: any, b?: string) => any): Function;
 
   once(eventType: string): Promise<DataSnapshot>;
 
@@ -576,7 +576,7 @@ export interface Query {
 
 export interface DataSnapshot {
   key: string;
-  ref: any; // TODO: Type it so that it returns a databaseReference.
+  // ref: any; // TODO: It's not properly typed and returns a native Ref which will lead to errors
   child(path: string): DataSnapshot;
 
   exists(): boolean;

src/firebase.ios.ts

@@ -1402,11 +1402,10 @@ class Query implements QueryBase {
     this.query = this.dbRef;
   }
 
-  on(eventType: string, callback: (a: any, b?: string) => any): Promise<any> {
+  on(eventType: string, callback: (a: any, b?: string) => any): Function {
     const onValueEvent = result => {
       callback(result);
     };
-    return new Promise((resolve, reject) => {
       try {
         if (eventType === "value" || eventType === "child_added" || eventType === "child_changed"
           || eventType === "child_removed" || eventType === "child_moved") {
@@ -1420,15 +1419,13 @@ class Query implements QueryBase {
           callback({
             error: "Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'"
           });
-          reject("Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'");
-          return;
         }
-        resolve();
       } catch (ex) {
         console.log("Error in firebase.on: " + ex);
-        reject(ex);
       }
-    });
+      finally {
+        return callback;
+      }
   }
 
   once(eventType: string): Promise<DataSnapshot> {
@@ -1838,7 +1835,7 @@ function nativeSnapshotToWebSnapshot(snapshot: FIRDataSnapshot): DataSnapshot {
 
   return {
     key: snapshot.key,
-    ref: snapshot.ref,
+    // ref: snapshot.ref,
     child: (path: string) => nativeSnapshotToWebSnapshot(snapshot.childSnapshotForPath(path)),
     exists: () => snapshot.exists(),
     forEach: (func: (datasnapshot) => any) => forEach(func),