From 87024b3732ed949c1b393fbb8ec546cf1da27572 Mon Sep 17 00:00:00 2001 From: cxam Date: Wed, 9 Jan 2019 19:52:07 +0000 Subject: [PATCH] Fix getAll function signature to allow array destructuring (#515) Fixes #501. --- dev/src/index.ts | 17 ++++++----- dev/src/transaction.ts | 17 ++++++----- dev/system-test/firestore.ts | 59 ++++++++++++++++++++++++++++++++++++ types/firestore.d.ts | 32 ++++++++++--------- 4 files changed, 95 insertions(+), 30 deletions(-) diff --git a/dev/src/index.ts b/dev/src/index.ts index 52b37e2c7..56483f652 100644 --- a/dev/src/index.ts +++ b/dev/src/index.ts @@ -687,9 +687,12 @@ export class Firestore { /** * Retrieves multiple documents from Firestore. * - * @param {DocumentReference} documentRef A `DocumentReference` to receive. - * @param {Array.} moreDocumentRefsOrReadOptions - * Additional `DocumentReferences` to receive, followed by an optional field + * The first argument is required and must be of type `DocumentReference` + * followed by any additional `DocumentReference` documents. If used, the + * optional `ReadOptions` must be the last argument. + * + * @param {Array.} documentRefsOrReadOptions + * The `DocumentReferences` to receive, followed by an optional field * mask. * @returns {Promise>} A Promise that * contains an array with the resulting document snapshots. @@ -703,14 +706,12 @@ export class Firestore { * console.log(`Second document: ${JSON.stringify(docs[1])}`); * }); */ - getAll( - documentRef: DocumentReference, - ...moreDocumentRefsOrReadOptions: Array): + getAll(...documentRefsOrReadOptions: Array): Promise { this._validator.minNumberOfArguments('Firestore.getAll', arguments, 1); - const {documents, fieldMask} = parseGetAllArguments( - this._validator, [documentRef, ...moreDocumentRefsOrReadOptions]); + const {documents, fieldMask} = + parseGetAllArguments(this._validator, documentRefsOrReadOptions); return this.getAll_(documents, fieldMask, requestTag()); } diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index dd2cf7333..e9d54f7e1 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -137,9 +137,12 @@ export class Transaction { * Retrieves multiple documents from Firestore. Holds a pessimistic lock on * all returned documents. * - * @param {DocumentReference} documentRef A `DocumentReference` to receive. - * @param {Array.} moreDocumentRefsOrReadOptions - * Additional `DocumentReferences` to receive, followed by an optional field + * The first argument is required and must be of type `DocumentReference` + * followed by any additional `DocumentReference` documents. If used, the + * optional `ReadOptions` must be the last argument. + * + * @param {Array.} documentRefsOrReadOptions + * The `DocumentReferences` to receive, followed by an optional field * mask. * @returns {Promise>} A Promise that * contains an array with the resulting document snapshots. @@ -157,9 +160,7 @@ export class Transaction { * }); * }); */ - getAll( - documentRef: DocumentReference, - ...moreDocumentRefsOrReadOptions: Array): + getAll(...documentRefsOrReadOptions: Array): Promise { if (!this._writeBatch.isEmpty) { throw new Error(READ_AFTER_WRITE_ERROR_MSG); @@ -167,8 +168,8 @@ export class Transaction { this._validator.minNumberOfArguments('Transaction.getAll', arguments, 1); - const {documents, fieldMask} = parseGetAllArguments( - this._validator, [documentRef, ...moreDocumentRefsOrReadOptions]); + const {documents, fieldMask} = + parseGetAllArguments(this._validator, documentRefsOrReadOptions); return this._firestore.getAll_( documents, fieldMask, this._requestTag, this._transactionId); diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index cda8b5d04..850858957 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -77,6 +77,18 @@ describe('Firestore class', () => { }); }); + it('getAll() supports array destructuring', () => { + const ref1 = randomCol.doc('doc1'); + const ref2 = randomCol.doc('doc2'); + return Promise.all([ref1.set({foo: 'a'}), ref2.set({foo: 'a'})]) + .then(() => { + return firestore.getAll(...[ref1, ref2]); + }) + .then(docs => { + expect(docs.length).to.equal(2); + }); + }); + it('getAll() supports field mask', () => { const ref1 = randomCol.doc('doc1'); return ref1.set({foo: 'a', bar: 'b'}) @@ -87,6 +99,19 @@ describe('Firestore class', () => { expect(docs[0].data()).to.deep.equal({foo: 'a'}); }); }); + + it('getAll() supports array destructuring with field mask', () => { + const ref1 = randomCol.doc('doc1'); + const ref2 = randomCol.doc('doc2'); + return Promise.all([ref1.set({f: 'a', b: 'b'}), ref2.set({f: 'a', b: 'b'})]) + .then(() => { + return firestore.getAll(...[ref1, ref2], {fieldMask: ['f']}); + }) + .then(docs => { + expect(docs[0].data()).to.deep.equal({f: 'a'}); + expect(docs[1].data()).to.deep.equal({f: 'a'}); + }); + }); }); describe('CollectionReference class', () => { @@ -1383,6 +1408,22 @@ describe('Transaction class', () => { }); }); + it('getAll() supports array destructuring', () => { + const ref1 = randomCol.doc('doc1'); + const ref2 = randomCol.doc('doc2'); + return Promise.all([ref1.set({}), ref2.set({})]) + .then(() => { + return firestore.runTransaction(updateFunction => { + return updateFunction.getAll(...[ref1, ref2]).then(docs => { + return Promise.resolve(docs.length); + }); + }); + }) + .then(res => { + expect(res).to.equal(2); + }); + }); + it('getAll() supports field mask', () => { const ref1 = randomCol.doc('doc1'); return ref1.set({foo: 'a', bar: 'b'}).then(() => { @@ -1397,6 +1438,24 @@ describe('Transaction class', () => { }); }); + it('getAll() supports array destructuring with field mask', () => { + const ref1 = randomCol.doc('doc1'); + const ref2 = randomCol.doc('doc2'); + return Promise.all([ref1.set({f: 'a', b: 'b'}), ref2.set({f: 'a', b: 'b'})]) + .then(() => { + return firestore + .runTransaction(updateFunction => { + return updateFunction + .getAll(...[ref1, ref2], {fieldMask: ['f']}) + .then((docs) => docs); + }) + .then(docs => { + expect(docs[0].data()).to.deep.equal({f: 'a'}); + expect(docs[1].data()).to.deep.equal({f: 'a'}); + }); + }); + }); + it('has get() with query', () => { const ref = randomCol.doc('doc'); const query = randomCol.where('foo', '==', 'bar'); diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 87f6e6b60..f79308dd2 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -140,16 +140,18 @@ declare namespace FirebaseFirestore { /** * Retrieves multiple documents from Firestore. * - * @param documentRef A `DocumentReference` to receive. - * @param moreDocumentRefsOrReadOptions Additional `DocumentReferences` to - * receive, followed by an optional field mask. + * The first argument is required and must be of type `DocumentReference` + * followed by any additional `DocumentReference` documents. If used, the + * optional `ReadOptions` must be the last argument. + * + * @param {Array.} documentRefsOrReadOptions + * The `DocumentReferences` to receive, followed by an optional field + * mask. * @return A Promise that resolves with an array of resulting document * snapshots. */ - getAll( - documentRef: DocumentReference, - ...moreDocumentRefsOrReadOptions: Array - ): Promise; + getAll(...documentRefsOrReadOptions: Array): + Promise; /** * Fetches the root collections that are associated with this Firestore @@ -260,16 +262,18 @@ declare namespace FirebaseFirestore { * Retrieves multiple documents from Firestore. Holds a pessimistic lock on * all returned documents. * - * @param documentRef A `DocumentReference` to receive. - * @param moreDocumentRefsOrReadOptions Additional `DocumentReferences` to - * receive, followed by an optional field mask. + * The first argument is required and must be of type `DocumentReference` + * followed by any additional `DocumentReference` documents. If used, the + * optional `ReadOptions` must be the last argument. + * + * @param {Array.} documentRefsOrReadOptions + * The `DocumentReferences` to receive, followed by an optional field + * mask. * @return A Promise that resolves with an array of resulting document * snapshots. */ - getAll( - documentRef: DocumentReference, - ...moreDocumentRefsOrReadOptions: Array - ): Promise; + getAll(...documentRefsOrReadOptions: Array): + Promise; /** * Create the document referred to by the provided `DocumentReference`.