Revert "Revert "Fix the type annotations in the RTDB Query.on/off/once API. (#1204)" (#1723)"

This reverts commit b6bfae0.

Author: schmidt-sebastian

packages/database-types/index.d.ts

@@ -71,17 +71,17 @@ export interface Query {
     eventType?: EventType,
     callback?: (a: DataSnapshot, b?: string | null) => any,
     context?: Object | null
-  ): any;
+  ): void;
   on(
     eventType: EventType,
-    callback: (a: DataSnapshot | null, b?: string) => any,
-    cancelCallbackOrContext?: Object | null,
+    callback: (a: DataSnapshot, b?: string | null) => any,
+    cancelCallbackOrContext?: ((a: Error) => any) | Object | null,
     context?: Object | null
-  ): (a: DataSnapshot | null, b?: string) => any;
+  ): (a: DataSnapshot, b?: string | null) => any;
   once(
     eventType: EventType,
-    successCallback?: (a: DataSnapshot, b?: string) => any,
-    failureCallbackOrContext?: Object | null,
+    successCallback?: (a: DataSnapshot, b?: string | null) => any,
+    failureCallbackOrContext?: ((a: Error) => void) | Object | null,
     context?: Object | null
   ): Promise<DataSnapshot>;
   orderByChild(path: string): Query;

packages/database/CHANGELOG.md

@@ -0,0 +1,3 @@
+# Unreleased
+- [changed] Improved consistency between the type annotations for `Query.on`/`Reference.on`, 
+  `Query.off`/`Reference.off` and `Query.once`/`Reference.once` (#1188, #1204).

packages/database/src/api/Query.ts

@@ -49,7 +49,7 @@ import { DataSnapshot } from './DataSnapshot';
 let __referenceConstructor: new (repo: Repo, path: Path) => Query;
 
 export interface SnapshotCallback {
-  (a: DataSnapshot, b?: string): any;
+  (a: DataSnapshot, b?: string | null): any;
 }
 
 /**
@@ -198,8 +198,8 @@ export class Query {
   on(
     eventType: string,
     callback: SnapshotCallback,
-    cancelCallbackOrContext?: ((a: Error) => any) | Object,
-    context?: Object
+    cancelCallbackOrContext?: ((a: Error) => any) | Object | null,
+    context?: Object | null
   ): SnapshotCallback {
     validateArgCount('Query.on', 2, 4, arguments.length);
     validateEventType('Query.on', 1, eventType, false);
@@ -264,7 +264,11 @@ export class Query {
    * @param {(function(!DataSnapshot, ?string=))=} callback
    * @param {Object=} context
    */
-  off(eventType?: string, callback?: SnapshotCallback, context?: Object) {
+  off(
+    eventType?: string,
+    callback?: SnapshotCallback,
+    context?: Object | null
+  ): void {
     validateArgCount('Query.off', 0, 3, arguments.length);
     validateEventType('Query.off', 1, eventType, true);
     validateCallback('Query.off', 2, callback, true);
@@ -293,23 +297,23 @@ export class Query {
    * Attaches a listener, waits for the first event, and then removes the listener
    * @param {!string} eventType
    * @param {!function(!DataSnapshot, string=)} userCallback
-   * @param cancelOrContext
+   * @param failureCallbackOrContext
    * @param context
    * @return {!firebase.Promise}
    */
   once(
     eventType: string,
     userCallback?: SnapshotCallback,
-    cancelOrContext?: ((a: Error) => void) | Object,
-    context?: Object
+    failureCallbackOrContext?: ((a: Error) => void) | Object | null,
+    context?: Object | null
   ): Promise<DataSnapshot> {
     validateArgCount('Query.once', 1, 4, arguments.length);
     validateEventType('Query.once', 1, eventType, false);
     validateCallback('Query.once', 2, userCallback, true);
 
     const ret = Query.getCancelAndContextArgs_(
       'Query.once',
-      cancelOrContext,
+      failureCallbackOrContext,
       context
     );
 
@@ -631,8 +635,8 @@ export class Query {
    */
   private static getCancelAndContextArgs_(
     fnName: string,
-    cancelOrContext?: ((a: Error) => void) | Object,
-    context?: Object
+    cancelOrContext?: ((a: Error) => void) | Object | null,
+    context?: Object | null
   ): { cancel: ((a: Error) => void) | null; context: Object | null } {
     const ret: {
       cancel: ((a: Error) => void) | null;

packages/database/src/core/view/Change.ts

@@ -32,7 +32,7 @@ export class Change {
     public snapshotNode: Node,
     public childName?: string,
     public oldSnap?: Node,
-    public prevName?: string
+    public prevName?: string | null
   ) {}
 
   /**

packages/firebase/index.d.ts

@@ -4388,8 +4388,8 @@ declare namespace firebase.database {
      * must also be called on any child listeners to remove the callback.
      *
      * If a callback is not specified, all callbacks for the specified eventType
-     * will be removed. Similarly, if no eventType or callback is specified, all
-     * callbacks for the `Reference` will be removed.
+     * will be removed. Similarly, if no eventType is specified, all callbacks
+     * for the `Reference` will be removed.
      *
      * @example
      * ```javascript
@@ -4414,16 +4414,17 @@ declare namespace firebase.database {
      * ```
      *
      * @param eventType One of the following strings: "value",
-     *   "child_added", "child_changed", "child_removed", or "child_moved."
-     * @param callback The
-     *   callback function that was passed to `on()`.
+     *   "child_added", "child_changed", "child_removed", or "child_moved." If
+     *   omitted, all callbacks for the `Reference` will be removed.
+     * @param callback The callback function that was passed to `on()` or
+     *   `undefined` to remove all callbacks.
      * @param context The context that was passed to `on()`.
      */
     off(
       eventType?: EventType,
       callback?: (a: firebase.database.DataSnapshot, b?: string | null) => any,
       context?: Object | null
-    ): any;
+    ): void;
 
     /**
      * Listens for data changes at a particular location.
@@ -4539,10 +4540,10 @@ declare namespace firebase.database {
      */
     on(
       eventType: EventType,
-      callback: (a: firebase.database.DataSnapshot | null, b?: string) => any,
+      callback: (a: firebase.database.DataSnapshot, b?: string | null) => any,
       cancelCallbackOrContext?: Object | null,
       context?: Object | null
-    ): (a: firebase.database.DataSnapshot | null, b?: string) => any;
+    ): (a: firebase.database.DataSnapshot | null, b?: string | null) => any;
 
     /**
      * Listens for exactly one event of the specified event type, and then stops
@@ -4579,10 +4580,13 @@ declare namespace firebase.database {
      */
     once(
       eventType: EventType,
-      successCallback?: (a: firebase.database.DataSnapshot, b?: string) => any,
-      failureCallbackOrContext?: Object | null,
+      successCallback?: (
+        a: firebase.database.DataSnapshot,
+        b?: string | null
+      ) => any,
+      failureCallbackOrContext?: ((a: Error) => void) | Object | null,
       context?: Object | null
-    ): Promise<DataSnapshot>;
+    ): Promise<firebase.database.DataSnapshot>;
     /**
      * Generates a new `Query` object ordered by the specified child key.
      *