From e8d476babf3c2ae9024308ca0d89eb0e4ab45431 Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Tue, 10 Mar 2020 15:16:26 -0700 Subject: [PATCH 1/2] feat: use QueryDocumentSnapshot in FirestoreDataConverter --- dev/src/document.ts | 25 ++++++++++++++++++++----- dev/src/reference.ts | 9 ++++++--- dev/src/types.ts | 15 ++++++--------- dev/test/typescript.ts | 4 ++-- dev/test/util/helpers.ts | 9 +++++++-- types/firestore.d.ts | 5 +++-- 6 files changed, 44 insertions(+), 23 deletions(-) diff --git a/dev/src/document.ts b/dev/src/document.ts index e0bc273c5..285bca4f3 100644 --- a/dev/src/document.ts +++ b/dev/src/document.ts @@ -25,7 +25,7 @@ import {FieldPath, validateFieldPath} from './path'; import {DocumentReference} from './reference'; import {Serializer} from './serializer'; import {Timestamp} from './timestamp'; -import {ApiMapValue, DocumentData, UpdateMap} from './types'; +import {ApiMapValue, defaultConverter, DocumentData, UpdateMap} from './types'; import {isEmpty, isObject, isPlainObject} from './util'; import api = google.firestore.v1; @@ -381,11 +381,26 @@ export class DocumentSnapshot { return undefined; } - const obj: DocumentData = {}; - for (const prop of Object.keys(fields)) { - obj[prop] = this._serializer.decodeValue(fields[prop]); + // We only want to use the converter and create a new QueryDocumentSnapshot + // if a converter has been provided. + if (this.ref._converter !== defaultConverter) { + const ref = new DocumentReference(this.ref.firestore, this.ref._path); + const snapshot = new DocumentSnapshotBuilder(ref); + snapshot.readTime = this.readTime; + snapshot.createTime = this.createTime; + snapshot.updateTime = this.updateTime; + snapshot.fieldsProto = this._fieldsProto; + + return this.ref._converter.fromFirestore( + snapshot.build() as QueryDocumentSnapshot + ); + } else { + const obj: DocumentData = {}; + for (const prop of Object.keys(fields)) { + obj[prop] = this._serializer.decodeValue(fields[prop]); + } + return obj as T; } - return this.ref._converter.fromFirestore(obj); } /** diff --git a/dev/src/reference.ts b/dev/src/reference.ts index c75a71e00..65948017c 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -558,8 +558,9 @@ export class DocumentReference implements Serializable { * return {title: post.title, author: post.author}; * }, * fromFirestore( - * data: FirebaseFirestore.DocumentData + * snapshot: FirebaseFirestore.QueryDocumentSnapshot * ): Post { + * const data = snapshot.data(); * return new Post(data.title, data.author); * } * }; @@ -2069,8 +2070,9 @@ export class Query { * return {title: post.title, author: post.author}; * }, * fromFirestore( - * data: FirebaseFirestore.DocumentData + * snapshot: FirebaseFirestore.QueryDocumentSnapshot * ): Post { + * const data = snapshot.data(); * return new Post(data.title, data.author); * } * }; @@ -2342,8 +2344,9 @@ export class CollectionReference extends Query { * return {title: post.title, author: post.author}; * }, * fromFirestore( - * data: FirebaseFirestore.DocumentData + * snapshot: FirebaseFirestore.QueryDocumentSnapshot * ): Post { + * const data = snapshot.data(); * return new Post(data.title, data.author); * } * }; diff --git a/dev/src/types.ts b/dev/src/types.ts index d318074b4..9914dabb1 100644 --- a/dev/src/types.ts +++ b/dev/src/types.ts @@ -22,6 +22,7 @@ import {FieldPath} from './path'; import {Timestamp} from './timestamp'; import api = google.firestore.v1; +import {QueryDocumentSnapshot} from './document'; /** * A map in the format of the Proto API @@ -113,7 +114,7 @@ export type RBTree = any; * return {title: post.title, author: post.author}; * }, * fromFirestore( - * data: FirebaseFirestore.DocumentData + * snapshot: FirebaseFirestore.QueryDocumentSnapshot * ): Post { * return new Post(data.title, data.author); * } @@ -142,7 +143,7 @@ export interface FirestoreDataConverter { * Called by the Firestore SDK to convert Firestore data into an object of * type T. */ - fromFirestore(data: DocumentData): T; + fromFirestore(snapshot: QueryDocumentSnapshot): T; } /** @@ -150,15 +151,11 @@ export interface FirestoreDataConverter { * @private */ export const defaultConverter: FirestoreDataConverter = { - toFirestore( - modelObject: FirebaseFirestore.DocumentData - ): FirebaseFirestore.DocumentData { + toFirestore(modelObject: DocumentData): DocumentData { return modelObject; }, - fromFirestore( - data: FirebaseFirestore.DocumentData - ): FirebaseFirestore.DocumentData { - return data; + fromFirestore(snapshot: QueryDocumentSnapshot): DocumentData { + return snapshot.data()!; }, }; diff --git a/dev/test/typescript.ts b/dev/test/typescript.ts index ee3845c9b..dddb21d42 100644 --- a/dev/test/typescript.ts +++ b/dev/test/typescript.ts @@ -58,8 +58,8 @@ xdescribe('firestore.d.ts', () => { toFirestore(modelObject: DocumentData): DocumentData { return modelObject; }, - fromFirestore(data: DocumentData): DocumentData { - return data; + fromFirestore(snapshot: QueryDocumentSnapshot): DocumentData { + return snapshot.data(); }, }; diff --git a/dev/test/util/helpers.ts b/dev/test/util/helpers.ts index 655b73d87..ef266f40d 100644 --- a/dev/test/util/helpers.ts +++ b/dev/test/util/helpers.ts @@ -19,7 +19,11 @@ import {Duplex} from 'stream'; import * as through2 from 'through2'; import * as proto from '../../protos/firestore_v1_proto_api'; -import {Firestore} from '../../src'; +import { + Firestore, + FirestoreDataConverter, + QueryDocumentSnapshot, +} from '../../src'; import {ClientPool} from '../../src/pool'; import {DocumentData, GapicClient} from '../../src/types'; @@ -348,7 +352,8 @@ export const postConverter = { toFirestore(post: Post): DocumentData { return {title: post.title, author: post.author}; }, - fromFirestore(data: DocumentData): Post { + fromFirestore(snapshot: QueryDocumentSnapshot): Post { + const data = snapshot.data(); return new Post(data.title, data.author); }, }; diff --git a/types/firestore.d.ts b/types/firestore.d.ts index d39713da9..aa58546b5 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -63,8 +63,9 @@ declare namespace FirebaseFirestore { * return {title: post.title, author: post.author}; * }, * fromFirestore( - * data: FirebaseFirestore.DocumentData + * snapshot: FirebaseFirestore.QueryDocumentSnapshot * ): Post { + * const data = snapshot.data(); * return new Post(data.title, data.author); * } * }; @@ -92,7 +93,7 @@ declare namespace FirebaseFirestore { * Called by the Firestore SDK to convert Firestore data into an object of * type T. */ - fromFirestore(data: DocumentData): T; + fromFirestore(snapshot: QueryDocumentSnapshot): T; } /** From e5daaed75aba8a30a1c3e3523ffe099abdd8997c Mon Sep 17 00:00:00 2001 From: Brian Chen Date: Wed, 11 Mar 2020 11:20:07 -0700 Subject: [PATCH 2/2] remove usage of builder when generating snapshot --- dev/src/document.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/dev/src/document.ts b/dev/src/document.ts index 285bca4f3..fc288fd5f 100644 --- a/dev/src/document.ts +++ b/dev/src/document.ts @@ -384,15 +384,18 @@ export class DocumentSnapshot { // We only want to use the converter and create a new QueryDocumentSnapshot // if a converter has been provided. if (this.ref._converter !== defaultConverter) { - const ref = new DocumentReference(this.ref.firestore, this.ref._path); - const snapshot = new DocumentSnapshotBuilder(ref); - snapshot.readTime = this.readTime; - snapshot.createTime = this.createTime; - snapshot.updateTime = this.updateTime; - snapshot.fieldsProto = this._fieldsProto; - + const untypedReference = new DocumentReference( + this.ref.firestore, + this.ref._path + ); return this.ref._converter.fromFirestore( - snapshot.build() as QueryDocumentSnapshot + new QueryDocumentSnapshot( + untypedReference, + this._fieldsProto!, + this.readTime, + this.createTime!, + this.updateTime! + ) ); } else { const obj: DocumentData = {};