Compat and @exp class for DataSnapshot

Author: schmidt-sebastian

common/api-review/firestore-exp.api.md

@@ -53,6 +53,8 @@ export class CollectionReference<T = DocumentData> extends Query<T> {
     get path(): string;
     // (undocumented)
     readonly type = "collection";
+    withConverter(converter: null): CollectionReference<DocumentData>;
+    // (undocumented)
     withConverter<U>(converter: FirestoreDataConverter<U>): CollectionReference<U>;
 }
 
@@ -101,6 +103,8 @@ export class DocumentReference<T = DocumentData> {
     get parent(): CollectionReference<T>;
     get path(): string;
     readonly type = "document";
+    withConverter(converter: null): DocumentReference<DocumentData>;
+    // (undocumented)
     withConverter<U>(converter: FirestoreDataConverter<U>): DocumentReference<U>;
 }
 
@@ -318,6 +322,8 @@ export class Query<T = DocumentData> {
     protected constructor();
     readonly firestore: FirebaseFirestore;
     readonly type: 'query' | 'collection';
+    withConverter(converter: null): Query<DocumentData>;
+    // (undocumented)
     withConverter<U>(converter: FirestoreDataConverter<U>): Query<U>;
 }
 

common/api-review/firestore-lite.api.md

@@ -47,6 +47,8 @@ export class CollectionReference<T = DocumentData> extends Query<T> {
     get path(): string;
     // (undocumented)
     readonly type = "collection";
+    withConverter(converter: null): CollectionReference<DocumentData>;
+    // (undocumented)
     withConverter<U>(converter: FirestoreDataConverter<U>): CollectionReference<U>;
 }
 
@@ -81,6 +83,8 @@ export class DocumentReference<T = DocumentData> {
     get parent(): CollectionReference<T>;
     get path(): string;
     readonly type = "document";
+    withConverter(converter: null): DocumentReference<DocumentData>;
+    // (undocumented)
     withConverter<U>(converter: FirestoreDataConverter<U>): DocumentReference<U>;
 }
 
@@ -195,6 +199,8 @@ export class Query<T = DocumentData> {
     protected constructor();
     readonly firestore: FirebaseFirestore;
     readonly type: 'query' | 'collection';
+    withConverter(converter: null): Query<DocumentData>;
+    // (undocumented)
     withConverter<U>(converter: FirestoreDataConverter<U>): Query<U>;
 }
 

packages/database/exp/index.ts

@@ -22,8 +22,48 @@ import { Component, ComponentType } from '@firebase/component';
 import { version } from '../package.json';
 import { FirebaseDatabase } from '../src/exp/Database';
 
-export { getDatabase, ServerValue } from '../src/exp/Database';
-export { enableLogging } from '../src/core/util/util';
+export {
+  enableLogging,
+  getDatabase,
+  goOffline,
+  goOnline,
+  ref,
+  refFromURL,
+  useDatabaseEmulator
+} from '../src/exp/Database';
+export {
+  OnDisconnect,
+  Reference,
+  ThenableReference
+} from '../src/exp/Reference';
+export { DataSnapshot } from '../src/exp/DataSnapshot';
+export {
+  ListenOptions,
+  Query,
+  QueryConstraint,
+  Unsubscribe,
+  endAt,
+  endBefore,
+  equalTo,
+  get,
+  limitToFirst,
+  limitToLast,
+  off,
+  onChildAdded,
+  onChildChanged,
+  onChildMoved,
+  onChildRemoved,
+  onValue,
+  orderByChild,
+  orderByKey,
+  orderByPriority,
+  orderByValue,
+  query,
+  startAfter,
+  startAt
+} from '../src/exp/Query';
+export { increment, serverTimestamp } from '../src/exp/ServerValue';
+export { runTransaction, TransactionOptions } from '../src/exp/Transaction';
 
 declare module '@firebase/component' {
   interface NameServiceMapping {

packages/database/src/api/DataSnapshot.ts

@@ -17,28 +17,19 @@
 
 import { validateArgCount, validateCallback } from '@firebase/util';
 import { validatePathString } from '../core/util/validation';
-import { Path } from '../core/util/Path';
-import { PRIORITY_INDEX } from '../core/snap/indexes/PriorityIndex';
-import { Node } from '../core/snap/Node';
 import { Reference } from './Reference';
-import { Index } from '../core/snap/indexes/Index';
-import { ChildrenNode } from '../core/snap/ChildrenNode';
+import { DataSnapshot as ExpDataSnapshot } from '../exp/DataSnapshot';
 
+// TODO(databaseexp): Import Compat from @firebase/util
+export interface Compat<T> {
+  readonly _delegate: T;
+}
 /**
  * Class representing a firebase data snapshot.  It wraps a SnapshotNode and
  * surfaces the public methods (val, forEach, etc.) we want to expose.
  */
-export class DataSnapshot {
-  /**
-   * @param node_ A SnapshotNode to wrap.
-   * @param ref_ The ref of the location this snapshot came from.
-   * @param index_ The iteration order for this snapshot
-   */
-  constructor(
-    private readonly node_: Node,
-    private readonly ref_: Reference,
-    private readonly index_: Index
-  ) {}
+export class DataSnapshot implements Compat<ExpDataSnapshot> {
+  constructor(readonly _delegate: ExpDataSnapshot) {}
 
   /**
    * Retrieves the snapshot contents as JSON.  Returns null if the snapshot is
@@ -48,7 +39,7 @@ export class DataSnapshot {
    */
   val(): unknown {
     validateArgCount('DataSnapshot.val', 0, 0, arguments.length);
-    return this.node_.val();
+    return this._delegate.val();
   }
 
   /**
@@ -58,15 +49,15 @@ export class DataSnapshot {
    */
   exportVal(): unknown {
     validateArgCount('DataSnapshot.exportVal', 0, 0, arguments.length);
-    return this.node_.val(true);
+    return this._delegate.exportVal();
   }
 
   // Do not create public documentation. This is intended to make JSON serialization work but is otherwise unnecessary
   // for end-users
   toJSON(): unknown {
     // Optional spacer argument is unnecessary because we're depending on recursion rather than stringifying the content
     validateArgCount('DataSnapshot.toJSON', 0, 1, arguments.length);
-    return this.exportVal();
+    return this._delegate.toJSON();
   }
 
   /**
@@ -76,7 +67,7 @@ export class DataSnapshot {
    */
   exists(): boolean {
     validateArgCount('DataSnapshot.exists', 0, 0, arguments.length);
-    return !this.node_.isEmpty();
+    return this._delegate.exists();
   }
 
   /**
@@ -90,14 +81,7 @@ export class DataSnapshot {
     // Ensure the childPath is a string (can be a number)
     childPathString = String(childPathString);
     validatePathString('DataSnapshot.child', 1, childPathString, false);
-
-    const childPath = new Path(childPathString);
-    const childRef = this.ref_.child(childPath);
-    return new DataSnapshot(
-      this.node_.getChild(childPath),
-      childRef,
-      PRIORITY_INDEX
-    );
+    return new DataSnapshot(this._delegate.child(childPathString));
   }
 
   /**
@@ -109,9 +93,7 @@ export class DataSnapshot {
   hasChild(childPathString: string): boolean {
     validateArgCount('DataSnapshot.hasChild', 1, 1, arguments.length);
     validatePathString('DataSnapshot.hasChild', 1, childPathString, false);
-
-    const childPath = new Path(childPathString);
-    return !this.node_.getChild(childPath).isEmpty();
+    return this._delegate.hasChild(childPathString);
   }
 
   /**
@@ -121,9 +103,7 @@ export class DataSnapshot {
    */
   getPriority(): string | number | null {
     validateArgCount('DataSnapshot.getPriority', 0, 0, arguments.length);
-
-    // typecast here because we never return deferred values or internal priorities (MAX_PRIORITY)
-    return this.node_.getPriority().val() as string | number | null;
+    return this._delegate.priority;
   }
 
   /**
@@ -134,21 +114,12 @@ export class DataSnapshot {
    * @return True if forEach was canceled by action returning true for
    * one of the child nodes.
    */
-  forEach(action: (d: DataSnapshot) => boolean | void): boolean {
+  forEach(action: (snapshot: DataSnapshot) => boolean | void): boolean {
     validateArgCount('DataSnapshot.forEach', 1, 1, arguments.length);
     validateCallback('DataSnapshot.forEach', 1, action, false);
-
-    if (this.node_.isLeafNode()) {
-      return false;
-    }
-
-    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)
-      );
-    });
+    return this._delegate.forEach(expDataSnapshot =>
+      action(new DataSnapshot(expDataSnapshot))
+    );
   }
 
   /**
@@ -157,16 +128,11 @@ export class DataSnapshot {
    */
   hasChildren(): boolean {
     validateArgCount('DataSnapshot.hasChildren', 0, 0, arguments.length);
-
-    if (this.node_.isLeafNode()) {
-      return false;
-    } else {
-      return !this.node_.isEmpty();
-    }
+    return this._delegate.hasChildren();
   }
 
   get key() {
-    return this.ref_.getKey();
+    return this._delegate.key;
   }
 
   /**
@@ -175,8 +141,7 @@ export class DataSnapshot {
    */
   numChildren(): number {
     validateArgCount('DataSnapshot.numChildren', 0, 0, arguments.length);
-
-    return this.node_.numChildren();
+    return this._delegate.size;
   }
 
   /**
@@ -185,8 +150,7 @@ export class DataSnapshot {
    */
   getRef(): Reference {
     validateArgCount('DataSnapshot.ref', 0, 0, arguments.length);
-
-    return this.ref_;
+    return new Reference(this._delegate.ref._repo, this._delegate.ref._path);
   }
 
   get ref() {

packages/database/src/api/Query.ts

@@ -15,6 +15,8 @@
  * limitations under the License.
  */
 
+import { DataSnapshot as ExpDataSnapshot } from '../exp/DataSnapshot';
+import { Reference as ExpReference } from '../exp/Reference';
 import {
   assert,
   Deferred,
@@ -45,6 +47,7 @@ import {
 import {
   ChildEventRegistration,
   EventRegistration,
+  ExpSnapshotCallback,
   ValueEventRegistration
 } from '../core/view/EventRegistration';
 
@@ -71,7 +74,7 @@ import { DataSnapshot } from './DataSnapshot';
 let __referenceConstructor: new (repo: Repo, path: Path) => Query;
 
 export interface SnapshotCallback {
-  (a: DataSnapshot, b?: string | null): unknown;
+  (dataSnapshot: DataSnapshot, previousChildName?: string | null): unknown;
 }
 
 /**
@@ -215,19 +218,19 @@ export class Query {
       cancelCallbackOrContext,
       context
     );
-
+    const expCallback = new ExpSnapshotCallback(callback);
     if (eventType === 'value') {
-      this.onValueEvent(callback, ret.cancel, ret.context);
+      this.onValueEvent(expCallback, ret.cancel, ret.context);
     } else {
-      const callbacks: { [k: string]: typeof callback } = {};
-      callbacks[eventType] = callback;
+      const callbacks: { [k: string]: ExpSnapshotCallback } = {};
+      callbacks[eventType] = expCallback;
       this.onChildEvent(callbacks, ret.cancel, ret.context);
     }
     return callback;
   }
 
   protected onValueEvent(
-    callback: (a: DataSnapshot) => void,
+    callback: ExpSnapshotCallback,
     cancelCallback: ((a: Error) => void) | null,
     context: object | null
   ) {
@@ -239,8 +242,8 @@ export class Query {
     repoAddEventCallbackForQuery(this.repo, this, container);
   }
 
-  onChildEvent(
-    callbacks: { [k: string]: SnapshotCallback },
+  protected onChildEvent(
+    callbacks: { [k: string]: ExpSnapshotCallback },
     cancelCallback: ((a: Error) => unknown) | null,
     context: object | null
   ) {
@@ -261,20 +264,20 @@ export class Query {
     validateEventType('Query.off', 1, eventType, true);
     validateCallback('Query.off', 2, callback, true);
     validateContextObject('Query.off', 3, context, true);
-
     let container: EventRegistration | null = null;
-    let callbacks: { [k: string]: typeof callback } | null = null;
+    let callbacks: { [k: string]: ExpSnapshotCallback } | null = null;
+
+    const expCallback = callback ? new ExpSnapshotCallback(callback) : null;
     if (eventType === 'value') {
-      const valueCallback = callback || null;
       container = new ValueEventRegistration(
-        valueCallback,
+        expCallback,
         null,
         context || null
       );
     } else if (eventType) {
       if (callback) {
         callbacks = {};
-        callbacks[eventType] = callback;
+        callbacks[eventType] = expCallback;
       }
       container = new ChildEventRegistration(callbacks, null, context || null);
     }
@@ -285,7 +288,15 @@ export class Query {
    * Get the server-value for this query, or return a cached value if not connected.
    */
   get(): Promise<DataSnapshot> {
-    return repoGetValue(this.repo, this);
+    return repoGetValue(this.repo, this).then(node => {
+      return new DataSnapshot(
+        new ExpDataSnapshot(
+          node,
+          new ExpReference(this.getRef().repo, this.getRef().path),
+          this.getQueryParams().getIndex()
+        )
+      );
+    });
   }
 
   /**