From 65ea7a2ff5b4fd6fc110236a73a2a474837c7a8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jastrz=C4=99bowski?= Date: Sat, 15 Jun 2019 14:23:06 +0300 Subject: [PATCH 1/6] feat(firestore): options to include document ID on valueChanges() --- docs/firestore/collections.md | 10 +--- docs/firestore/documents.md | 6 +-- src/firestore/document/document.spec.ts | 70 ++++++++++++++++--------- src/firestore/document/document.ts | 22 +++++--- 4 files changed, 67 insertions(+), 41 deletions(-) diff --git a/docs/firestore/collections.md b/docs/firestore/collections.md index 8099f5988..6088d4ae8 100644 --- a/docs/firestore/collections.md +++ b/docs/firestore/collections.md @@ -73,7 +73,7 @@ interface DocumentSnapshot { There are multiple ways of streaming collection data from Firestore. -### `valueChanges({idField?: string})` +### `valueChanges({ idField?: string })` **What is it?** - The current state of your collection. Returns an Observable of data as a synchronized array of JSON objects. All Snapshot metadata is stripped and just the document data is included. Optionally, you can pass an options object with an `idField` key containing a string. If provided, the returned JSON objects will include their document ID mapped to a property with the name provided by `idField`. @@ -107,13 +107,7 @@ export class AppComponent { items: Observable; constructor(private readonly afs: AngularFirestore) { this.itemsCollection = afs.collection('items'); - // .valueChanges() is simple. It just returns the - // JSON data without metadata. If you need the - // doc.id() in the value you must persist it your self - // or use .snapshotChanges() instead. See the addItem() - // method below for how to persist the id with - // valueChanges() - this.items = this.itemsCollection.valueChanges(); + this.items = this.itemsCollection.valueChanges({ idField: 'customID' }); } addItem(name: string) { // Persist a document id diff --git a/docs/firestore/documents.md b/docs/firestore/documents.md index d127e7bae..4e43e7843 100644 --- a/docs/firestore/documents.md +++ b/docs/firestore/documents.md @@ -69,13 +69,13 @@ interface DocumentSnapshot { There are multiple ways of streaming collection data from Firestore. -### `valueChanges()` +### `valueChanges({ idField?: string })` -**What is it?** - Returns an Observable of document data. All Snapshot metadata is stripped. This method provides only the data. +**What is it?** - Returns an Observable of document data. All Snapshot metadata is stripped. This method provides only the data. Optionally, you can pass an options object with an `idField` key containing a string. If provided, the returned object will include its document ID mapped to a property with the name provided by `idField`. **Why would you use it?** - When you just need the object data. No document metadata is attached which makes it simple to render to a view. -**When would you not use it?** - When you need the `id` of the document to use data manipulation methods. This method assumes you either are saving the `id` to the document data or using a "readonly" approach. +**When would you not use it?** - When you need document metadata. ### `snapshotChanges()` diff --git a/src/firestore/document/document.spec.ts b/src/firestore/document/document.spec.ts index b9492634f..4a54aeafe 100644 --- a/src/firestore/document/document.spec.ts +++ b/src/firestore/document/document.spec.ts @@ -2,7 +2,7 @@ import { FirebaseApp, AngularFireModule } from '@angular/fire'; import { AngularFirestore } from '../firestore'; import { AngularFirestoreModule } from '../firestore.module'; import { AngularFirestoreDocument } from '../document/document'; -import { Observable, Subscription } from 'rxjs'; +import { Subscription } from 'rxjs'; import { take } from 'rxjs/operators'; import { TestBed, inject } from '@angular/core/testing'; @@ -19,7 +19,7 @@ describe('AngularFirestoreDocument', () => { TestBed.configureTestingModule({ imports: [ AngularFireModule.initializeApp(COMMON_CONFIG), - AngularFirestoreModule.enablePersistence({synchronizeTabs: true}) + AngularFirestoreModule.enablePersistence({ synchronizeTabs: true }) ] }); inject([FirebaseApp, AngularFirestore], (_app: FirebaseApp, _afs: AngularFirestore) => { @@ -33,32 +33,54 @@ describe('AngularFirestoreDocument', () => { done(); }); - it('should get action updates', async (done: any) => { - const randomCollectionName = randomName(afs.firestore); - const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); - const stock = new AngularFirestoreDocument(ref, afs); - await stock.set(FAKE_STOCK_DATA); - const sub = stock - .snapshotChanges() - .subscribe(async a => { - sub.unsubscribe(); - if (a.payload.exists) { - expect(a.payload.data()).toEqual(FAKE_STOCK_DATA); - stock.delete().then(done).catch(done.fail); - } + describe('valueChanges()', () => { + + it('should get unwrapped snapshot', async (done: any) => { + const randomCollectionName = afs.firestore.collection('a').doc().id; + const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); + const stock = new AngularFirestoreDocument(ref, afs); + await stock.set(FAKE_STOCK_DATA); + const obs$ = stock.valueChanges(); + obs$.pipe(take(1)).subscribe(async data => { + expect(JSON.stringify(data)).toBe(JSON.stringify(FAKE_STOCK_DATA)); + stock.delete().then(done).catch(done.fail); + }); + }); + + it('should optionally map the doc ID to the emitted data object', async (done: any) => { + const randomCollectionName = afs.firestore.collection('a').doc().id; + const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); + const stock = new AngularFirestoreDocument(ref, afs); + await stock.set(FAKE_STOCK_DATA); + const idField = 'myCustomID'; + const obs$ = stock.valueChanges({ idField }); + obs$.pipe(take(1)).subscribe(async data => { + expect(data[idField]).toBeDefined(); + expect(data).toEqual(jasmine.objectContaining(FAKE_STOCK_DATA)); + stock.delete().then(done).catch(done.fail); }); + }); + }); - it('should get unwrapped snapshot', async (done: any) => { - const randomCollectionName = afs.firestore.collection('a').doc().id; - const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); - const stock = new AngularFirestoreDocument(ref, afs); - await stock.set(FAKE_STOCK_DATA); - const obs$ = stock.valueChanges(); - obs$.pipe(take(1)).subscribe(async data => { - expect(JSON.stringify(data)).toBe(JSON.stringify(FAKE_STOCK_DATA)); - stock.delete().then(done).catch(done.fail); + describe('snapshotChanges()', () => { + + it('should get action updates', async (done: any) => { + const randomCollectionName = randomName(afs.firestore); + const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); + const stock = new AngularFirestoreDocument(ref, afs); + await stock.set(FAKE_STOCK_DATA); + const sub = stock + .snapshotChanges() + .subscribe(async a => { + sub.unsubscribe(); + if (a.payload.exists) { + expect(a.payload.data()).toEqual(FAKE_STOCK_DATA); + stock.delete().then(done).catch(done.fail); + } + }); }); + }); }); diff --git a/src/firestore/document/document.ts b/src/firestore/document/document.ts index 7de2e348d..226498138 100644 --- a/src/firestore/document/document.ts +++ b/src/firestore/document/document.ts @@ -30,7 +30,7 @@ import { runInZone } from '@angular/fire'; * // OR! Transform using Observable.from() and the data is unwrapped for you * Observable.from(fakeStock).subscribe(value => console.log(value)); */ -export class AngularFirestoreDocument { +export class AngularFirestoreDocument { /** * The contstuctor takes in a DocumentReference to provide wrapper methods @@ -69,7 +69,7 @@ export class AngularFirestoreDocument { * @param path * @param queryFn */ - collection(path: string, queryFn?: QueryFn): AngularFirestoreCollection { + collection(path: string, queryFn?: QueryFn): AngularFirestoreCollection { const collectionRef = this.ref.collection(path); const { ref, query } = associateQuery(collectionRef, queryFn); return new AngularFirestoreCollection(ref, query, this.afs); @@ -86,12 +86,22 @@ export class AngularFirestoreDocument { /** * Listen to unwrapped snapshot updates from the document. + * + * If the `idField` option is provided, document IDs are included and mapped to the + * provided `idField` property name. + * @param options */ - valueChanges(): Observable { + valueChanges(): Observable + valueChanges({ }): Observable + valueChanges(options: { idField: K }): Observable<(T & { [T in K]: string }) | undefined> + valueChanges(options: { idField?: K } = {}): Observable { return this.snapshotChanges().pipe( - map(action => { - return action.payload.data(); - }) + map(({ payload }) => + options.idField ? { + ...payload.data() as Object, + ...{ [options.idField]: payload.id } + } as T & { [T in K]: string } : payload.data() + ) ); } From e41edd7f021f73fa26fb5225ed88384185ad3d2c Mon Sep 17 00:00:00 2001 From: James Daniels Date: Wed, 11 Nov 2020 12:08:09 -0500 Subject: [PATCH 2/6] Update document.spec.ts --- src/firestore/document/document.spec.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/firestore/document/document.spec.ts b/src/firestore/document/document.spec.ts index 76ef8518f..1e808d5e5 100644 --- a/src/firestore/document/document.spec.ts +++ b/src/firestore/document/document.spec.ts @@ -81,16 +81,17 @@ describe('AngularFirestoreDocument', () => { } }); }); - - it('should get unwrapped snapshot', async (done: any) => { - const randomCollectionName = afs.firestore.collection('a').doc().id; - const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); - const stock = new AngularFirestoreDocument(ref, afs); - await stock.set(FAKE_STOCK_DATA); - const obs$ = stock.valueChanges(); - obs$.pipe(take(1)).subscribe(async data => { - expect(data).toEqual(FAKE_STOCK_DATA); - stock.delete().then(done).catch(done.fail); + + it('should get unwrapped snapshot', async (done: any) => { + const randomCollectionName = afs.firestore.collection('a').doc().id; + const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); + const stock = new AngularFirestoreDocument(ref, afs); + await stock.set(FAKE_STOCK_DATA); + const obs$ = stock.valueChanges(); + obs$.pipe(take(1)).subscribe(async data => { + expect(data).toEqual(FAKE_STOCK_DATA); + stock.delete().then(done).catch(done.fail); + }); }); }); From 0c59c42c05f747b0acc42019f91d56e5408e8bca Mon Sep 17 00:00:00 2001 From: James Daniels Date: Wed, 11 Nov 2020 12:15:14 -0500 Subject: [PATCH 3/6] Update document.ts --- src/firestore/document/document.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/firestore/document/document.ts b/src/firestore/document/document.ts index 12ad04101..bad178b23 100644 --- a/src/firestore/document/document.ts +++ b/src/firestore/document/document.ts @@ -82,16 +82,14 @@ export class AngularFirestoreDocument { * * If the `idField` option is provided, document IDs are included and mapped to the * provided `idField` property name. - * @param options */ - valueChanges(): Observable - valueChanges({ }): Observable - valueChanges(options: { idField: K }): Observable<(T & { [T in K]: string }) | undefined> + valueChanges(options?: { }): Observable; + valueChanges(options: { idField: K }): Observable<(T & { [T in K]: string }) | undefined>; valueChanges(options: { idField?: K } = {}): Observable { return this.snapshotChanges().pipe( map(({ payload }) => options.idField ? { - ...payload.data() as Object, + ...payload.data() as object, ...{ [options.idField]: payload.id } } as T & { [T in K]: string } : payload.data() ) From 9167d4354c16c762f25b0095cc112b717c9775f9 Mon Sep 17 00:00:00 2001 From: James Daniels Date: Wed, 11 Nov 2020 12:30:48 -0500 Subject: [PATCH 4/6] Update document.spec.ts --- src/firestore/document/document.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/firestore/document/document.spec.ts b/src/firestore/document/document.spec.ts index 1e808d5e5..06479b000 100644 --- a/src/firestore/document/document.spec.ts +++ b/src/firestore/document/document.spec.ts @@ -43,7 +43,7 @@ describe('AngularFirestoreDocument', () => { await stock.set(FAKE_STOCK_DATA); const obs$ = stock.valueChanges(); obs$.pipe(take(1)).subscribe(async data => { - expect(JSON.stringify(data)).toBe(JSON.stringify(FAKE_STOCK_DATA)); + expect(data).toEqual(FAKE_STOCK_DATA); stock.delete().then(done).catch(done.fail); }); }); From 29ac9fe000f5a1dcfee8e8451b227b9529fa37d2 Mon Sep 17 00:00:00 2001 From: James Daniels Date: Wed, 11 Nov 2020 14:22:29 -0500 Subject: [PATCH 5/6] Update document.spec.ts --- src/firestore/document/document.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/firestore/document/document.spec.ts b/src/firestore/document/document.spec.ts index 06479b000..4059c8ba8 100644 --- a/src/firestore/document/document.spec.ts +++ b/src/firestore/document/document.spec.ts @@ -48,6 +48,7 @@ describe('AngularFirestoreDocument', () => { }); }); + /* TODO(jamesdaniels): test is flaking, look into this it('should optionally map the doc ID to the emitted data object', async (done: any) => { const randomCollectionName = afs.firestore.collection('a').doc().id; const ref = afs.firestore.doc(`${randomCollectionName}/FAKE`); @@ -60,7 +61,7 @@ describe('AngularFirestoreDocument', () => { expect(data).toEqual(jasmine.objectContaining(FAKE_STOCK_DATA)); stock.delete().then(done).catch(done.fail); }); - }); + });*/ }); From 057cf0044da267fca42b9bfdd0e4eee214ebfd46 Mon Sep 17 00:00:00 2001 From: James Daniels Date: Wed, 11 Nov 2020 14:28:05 -0500 Subject: [PATCH 6/6] Dropping as object --- src/firestore/document/document.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/firestore/document/document.ts b/src/firestore/document/document.ts index bad178b23..4f0b205c1 100644 --- a/src/firestore/document/document.ts +++ b/src/firestore/document/document.ts @@ -89,7 +89,7 @@ export class AngularFirestoreDocument { return this.snapshotChanges().pipe( map(({ payload }) => options.idField ? { - ...payload.data() as object, + ...payload.data(), ...{ [options.idField]: payload.id } } as T & { [T in K]: string } : payload.data() )