Review feedback

Author: schmidt-sebastian

packages/database-types/index.d.ts

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

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): void {
+  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/database/src/core/view/Event.ts

@@ -66,7 +66,7 @@ export class DataEvent implements Event {
       | ' child_removed',
     public eventRegistration: EventRegistration,
     public snapshot: DataSnapshot,
-    public prevName?: string
+    public prevName?: string | null
   ) {}
 
   /**

packages/firebase/index.d.ts

@@ -4373,8 +4373,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
@@ -4399,16 +4399,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.
@@ -4524,10 +4525,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
@@ -4564,10 +4565,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.
      *