From 4eefcf178b213b9003926e8802bbffccad68555c Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 15 Sep 2023 18:54:06 -0400 Subject: [PATCH] PR Feedback and fix types. --- dev/conformance/runner.ts | 2 +- dev/src/document-change.ts | 2 +- dev/src/document.ts | 23 +++++------ dev/src/reference.ts | 81 +++++++++++++++++++------------------- dev/src/transaction.ts | 11 ++++-- dev/src/write-batch.ts | 11 ++---- dev/test/document.ts | 10 ++--- types/firestore.d.ts | 79 ++++++++++++++++++++++++------------- 8 files changed, 119 insertions(+), 100 deletions(-) diff --git a/dev/conformance/runner.ts b/dev/conformance/runner.ts index 02b1a7595..947142897 100644 --- a/dev/conformance/runner.ts +++ b/dev/conformance/runner.ts @@ -193,7 +193,7 @@ const convertInput = { } return args; }, - snapshot: (snapshot: ConformanceProto) => { + snapshot: (snapshot: ConformanceProto): QuerySnapshot => { const docs: QueryDocumentSnapshot[] = []; const changes: DocumentChange[] = []; const readTime = Timestamp.fromProto(snapshot.readTime); diff --git a/dev/src/document-change.ts b/dev/src/document-change.ts index 02a9868b8..22822702e 100644 --- a/dev/src/document-change.ts +++ b/dev/src/document-change.ts @@ -183,7 +183,7 @@ export class DocumentChange< * @param {*} other The value to compare against. * @return true if this `DocumentChange` is equal to the provided value. */ - isEqual(other: unknown): boolean { + isEqual(other: firestore.DocumentChange): boolean { if (this === other) { return true; } diff --git a/dev/src/document.ts b/dev/src/document.ts index 1fee04ed7..84a77f2a4 100644 --- a/dev/src/document.ts +++ b/dev/src/document.ts @@ -23,14 +23,13 @@ import * as assert from 'assert'; import {google} from '../protos/firestore_v1_proto_api'; import {FieldTransform} from './field-value'; import {FieldPath, validateFieldPath} from './path'; -import {DocumentReference, validateDocumentReference} from './reference'; +import {DocumentReference} from './reference'; import {Serializer} from './serializer'; import {Timestamp} from './timestamp'; import {ApiMapValue, defaultConverter, UpdateMap} from './types'; import {isEmpty, isObject, isPlainObject} from './util'; import api = google.firestore.v1; -import {Firestore} from './index'; /** * Returns a builder for DocumentSnapshot and QueryDocumentSnapshot instances. @@ -158,14 +157,11 @@ export class DocumentSnapshot< * @return The created DocumentSnapshot. */ static fromObject( - ref: firestore.DocumentReference, + ref: DocumentReference, obj: firestore.DocumentData ): DocumentSnapshot { - const serializer = (ref.firestore as Firestore)._serializer!; - return new DocumentSnapshot( - validateDocumentReference('documentRef', ref), - serializer.encodeFields(obj) - ); + const serializer = ref.firestore._serializer!; + return new DocumentSnapshot(ref, serializer.encodeFields(obj)); } /** * Creates a DocumentSnapshot from an UpdateMap. @@ -440,7 +436,7 @@ export class DocumentSnapshot< this.readTime, this.createTime!, this.updateTime! - ) as firestore.QueryDocumentSnapshot + ) ); } else { const obj: firestore.DocumentData = {}; @@ -555,7 +551,9 @@ export class DocumentSnapshot< * @return {boolean} true if this `DocumentSnapshot` is equal to the provided * value. */ - isEqual(other: unknown): boolean { + isEqual( + other: firestore.DocumentSnapshot + ): boolean { // Since the read time is different on every document read, we explicitly // ignore all document metadata in this comparison. return ( @@ -986,7 +984,10 @@ export class DocumentTransform< updateMap.set(new FieldPath(prop), obj[prop]); } - return DocumentTransform.fromUpdateMap(ref, updateMap); + return DocumentTransform.fromUpdateMap( + ref, + updateMap + ); } /** diff --git a/dev/src/reference.ts b/dev/src/reference.ts index cb950677f..0e7ebc57d 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -334,9 +334,7 @@ export class DocumentReference< * }); * ``` */ - listCollections(): Promise< - Array> - > { + listCollections(): Promise> { const tag = requestTag(); return this.firestore.initializeIfNeeded(tag).then(() => { const request: api.IListCollectionIdsRequest = { @@ -353,9 +351,7 @@ export class DocumentReference< tag ) .then(collectionIds => { - const collections: Array< - CollectionReference - > = []; + const collections: Array = []; // We can just sort this list using the default comparator since it // will only contain collection ids. @@ -602,7 +598,9 @@ export class DocumentReference< * @return {boolean} true if this `DocumentReference` is equal to the provided * value. */ - isEqual(other: unknown): boolean { + isEqual( + other: firestore.DocumentReference + ): boolean { return ( this === other || (other instanceof DocumentReference && @@ -738,7 +736,7 @@ abstract class FilterInternal { /** Returns the proto representation of this filter */ abstract toProto(): Filter; - abstract isEqual(other: unknown): boolean; + abstract isEqual(other: FilterInternal): boolean; } class CompositeFilterInternal extends FilterInternal { @@ -1158,7 +1156,7 @@ export class QuerySnapshot< * @return {boolean} true if this `QuerySnapshot` is equal to the provided * value. */ - isEqual(other: unknown): boolean { + isEqual(other: firestore.QuerySnapshot): boolean { // Since the read time is different on every query read, we explicitly // ignore all metadata in this comparison. @@ -1252,7 +1250,7 @@ export class QueryOptions< */ static forCollectionGroupQuery< AppModelType, - DbModelType extends firestore.DocumentData = firestore.DocumentData + DbModelType extends firestore.DocumentData >( collectionId: string, converter: firestore.FirestoreDataConverter< @@ -1374,7 +1372,7 @@ export class QueryOptions< return this.fieldOrders.length > 0; } - isEqual(other: unknown): boolean { + isEqual(other: QueryOptions): boolean { if (this === other) { return true; } @@ -1531,9 +1529,9 @@ export class Query< * ``` */ where( - fieldPath: string | firestore.FieldPath, + fieldPath: string | FieldPath, opStr: firestore.WhereFilterOp, - value: unknown + value: any ): Query; /** @@ -1558,7 +1556,7 @@ export class Query< * }); * ``` */ - where(filter: firestore.Filter): Query; + where(filter: Filter): Query; where( fieldPathOrFilter: string | FieldPath | Filter, @@ -2096,7 +2094,9 @@ export class Query< * ``` */ startAt( - ...fieldValuesOrDocumentSnapshot: unknown[] + ...fieldValuesOrDocumentSnapshot: Array< + firestore.DocumentSnapshot | unknown + > ): Query { validateMinNumberOfArguments( 'Query.startAt', @@ -2140,7 +2140,9 @@ export class Query< * ``` */ startAfter( - ...fieldValuesOrDocumentSnapshot: Array + ...fieldValuesOrDocumentSnapshot: Array< + firestore.DocumentSnapshot | unknown + > ): Query { validateMinNumberOfArguments( 'Query.startAfter', @@ -2183,7 +2185,9 @@ export class Query< * ``` */ endBefore( - ...fieldValuesOrDocumentSnapshot: Array + ...fieldValuesOrDocumentSnapshot: Array< + firestore.DocumentSnapshot | unknown + > ): Query { validateMinNumberOfArguments( 'Query.endBefore', @@ -2226,7 +2230,9 @@ export class Query< * ``` */ endAt( - ...fieldValuesOrDocumentSnapshot: Array + ...fieldValuesOrDocumentSnapshot: Array< + firestore.DocumentSnapshot | unknown + > ): Query { validateMinNumberOfArguments( 'Query.endAt', @@ -2683,9 +2689,7 @@ export class Query< * ``` */ onSnapshot( - onNext: ( - snapshot: firestore.QuerySnapshot - ) => void, + onNext: (snapshot: QuerySnapshot) => void, onError?: (error: Error) => void ): () => void { validateFunction('onNext', onNext); @@ -2844,7 +2848,7 @@ export class CollectionReference< constructor( firestore: Firestore, path: ResourcePath, - converter = defaultConverter() + converter?: firestore.FirestoreDataConverter ) { super(firestore, QueryOptions.forCollectionQuery(path, converter)); } @@ -2892,7 +2896,7 @@ export class CollectionReference< * console.log(`Parent name: ${documentRef.path}`); * ``` */ - get parent(): DocumentReference | null { + get parent(): DocumentReference | null { if (this._queryOptions.parentPath.isDocument) { return new DocumentReference( this.firestore, @@ -3176,7 +3180,7 @@ export class AggregateQuery< ) {} /** The query whose aggregations will be calculated by this object. */ - get query(): firestore.Query { + get query(): Query { return this._query; } @@ -3239,14 +3243,7 @@ export class AggregateQuery< if (proto.result) { const readTime = Timestamp.fromProto(proto.readTime!); const data = this.decodeResult(proto.result); - callback( - undefined, - new AggregateQuerySnapshot< - AggregateSpecType, - AppModelType, - DbModelType - >(this, readTime, data) - ); + callback(undefined, new AggregateQuerySnapshot(this, readTime, data)); } else { callback(Error('RunAggregationQueryResponse is missing result')); } @@ -3377,7 +3374,13 @@ export class AggregateQuery< * @return `true` if this object is "equal" to the given object, as * defined above, or `false` otherwise. */ - isEqual(other: firestore.AggregateQuery): boolean { + isEqual( + other: firestore.AggregateQuery< + AggregateSpecType, + AppModelType, + DbModelType + > + ): boolean { if (this === other) { return true; } @@ -3396,8 +3399,8 @@ export class AggregateQuery< */ export class AggregateQuerySnapshot< AggregateSpecType extends firestore.AggregateSpec, - AppModelType, - DbModelType extends firestore.DocumentData + AppModelType = firestore.DocumentData, + DbModelType extends firestore.DocumentData = firestore.DocumentData > implements firestore.AggregateQuerySnapshot< AggregateSpecType, @@ -3425,16 +3428,12 @@ export class AggregateQuerySnapshot< ) {} /** The query that was executed to produce this result. */ - get query(): firestore.AggregateQuery< - AggregateSpecType, - AppModelType, - DbModelType - > { + get query(): AggregateQuery { return this._query; } /** The time this snapshot was read. */ - get readTime(): firestore.Timestamp { + get readTime(): Timestamp { return this._readTime; } diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 2b8526d1c..c57dbf35c 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -100,7 +100,7 @@ export class Transaction implements firestore.Transaction { * @return {Promise} A DocumentSnapshot for the read data. */ get( - documentRef: DocumentReference + documentRef: firestore.DocumentReference ): Promise>; /** @@ -413,7 +413,10 @@ export class Transaction implements firestore.Transaction { documentRef: DocumentReference, precondition?: firestore.Precondition ): this { - this._writeBatch.delete(documentRef, precondition); + this._writeBatch.delete( + documentRef, + precondition + ); return this; } @@ -584,7 +587,7 @@ export class Transaction implements firestore.Transaction { */ export function parseGetAllArguments< AppModelType, - DbModelType extends firestore.DocumentData = firestore.DocumentData + DbModelType extends firestore.DocumentData >( documentRefsOrReadOptions: Array< | firestore.DocumentReference @@ -621,7 +624,7 @@ export function parseGetAllArguments< } for (let i = 0; i < documents.length; ++i) { - validateDocumentReference(i, documents[i]); + validateDocumentReference(i, documents[i]); } validateReadOptions('options', readOptions, {optional: true}); diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts index f227526fd..df90b5f28 100644 --- a/dev/src/write-batch.ts +++ b/dev/src/write-batch.ts @@ -89,7 +89,7 @@ export class WriteResult implements firestore.WriteResult { * @param {*} other The value to compare against. * @return true if this `WriteResult` is equal to the provided value. */ - isEqual(other: unknown): boolean { + isEqual(other: firestore.WriteResult): boolean { return ( this === other || (other instanceof WriteResult && @@ -331,10 +331,7 @@ export class WriteBatch implements firestore.WriteBatch { const ref = validateDocumentReference('documentRef', documentRef); let firestoreData: firestore.DocumentData; if (mergeLeaves || mergePaths) { - // Cast to any in order to satisfy the union type constraint on - // toFirestore(). - // eslint-disable-next-line @typescript-eslint/no-explicit-any - firestoreData = (ref._converter as any).toFirestore(data, options); + firestoreData = ref._converter.toFirestore(data, options); } else { firestoreData = ref._converter.toFirestore(data as AppModelType); } @@ -356,11 +353,11 @@ export class WriteBatch implements firestore.WriteBatch { firestoreData = documentMask.applyTo(firestoreData); } - const transform = DocumentTransform.fromObject(documentRef, firestoreData); + const transform = DocumentTransform.fromObject(ref, firestoreData); transform.validate(); const op: PendingWriteOp = () => { - const document = DocumentSnapshot.fromObject(documentRef, firestoreData); + const document = DocumentSnapshot.fromObject(ref, firestoreData); if (mergePaths) { documentMask!.removeFields(transform.fields); diff --git a/dev/test/document.ts b/dev/test/document.ts index 2eee77287..db4f86c65 100644 --- a/dev/test/document.ts +++ b/dev/test/document.ts @@ -2073,13 +2073,9 @@ describe('update document', () => { }).to.throw(INVALID_ARGUMENTS_TO_UPDATE); expect(() => { - firestore.doc('collectionId/documentId').update( - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - {foo: 'bar'}, - {exists: true}, - 'foo' - ); + firestore + .doc('collectionId/documentId') + .update({foo: 'bar'}, {exists: true}, 'foo'); }).to.throw(INVALID_ARGUMENTS_TO_UPDATE); }); diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 7e8fa418f..5613d90c8 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -578,7 +578,7 @@ declare namespace FirebaseFirestore { * @param other The `GeoPoint` to compare against. * @return true if this `GeoPoint` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: GeoPoint): boolean; } /** @@ -1217,7 +1217,7 @@ declare namespace FirebaseFirestore { * @param other The `WriteResult` to compare against. * @return true if this `WriteResult` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: WriteResult): boolean; } /** @@ -1259,14 +1259,14 @@ declare namespace FirebaseFirestore { * @param collectionPath A slash-separated path to a collection. * @return The `CollectionReference` instance. */ - collection(collectionPath: string): CollectionReference; + collection(collectionPath: string): CollectionReference; /** * Fetches the subcollections that are direct children of this document. * * @returns A Promise that resolves with an array of CollectionReferences. */ - listCollections(): Promise>>; + listCollections(): Promise>; /** * Creates a document referred to by this `DocumentReference` with the @@ -1381,7 +1381,7 @@ declare namespace FirebaseFirestore { * @param other The `DocumentReference` to compare against. * @return true if this `DocumentReference` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: DocumentReference): boolean; /** * Applies a custom data converter to this DocumentReference, allowing you @@ -1562,7 +1562,7 @@ declare namespace FirebaseFirestore { where( fieldPath: string | FieldPath, opStr: WhereFilterOp, - value: unknown + value: any ): Query; /** @@ -1659,7 +1659,9 @@ declare namespace FirebaseFirestore { * @param snapshot The snapshot of the document to start after. * @return The created Query. */ - startAt(snapshot: DocumentSnapshot): Query; + startAt( + snapshot: DocumentSnapshot + ): Query; /** * Creates and returns a new Query that starts at the provided fields @@ -1670,7 +1672,7 @@ declare namespace FirebaseFirestore { * of the query's order by. * @return The created Query. */ - startAt(...fieldValues: unknown[]): Query; + startAt(...fieldValues: any[]): Query; /** * Creates and returns a new Query that starts after the provided document @@ -1681,7 +1683,9 @@ declare namespace FirebaseFirestore { * @param snapshot The snapshot of the document to start after. * @return The created Query. */ - startAfter(snapshot: DocumentSnapshot): Query; + startAfter( + snapshot: DocumentSnapshot + ): Query; /** * Creates and returns a new Query that starts after the provided fields @@ -1703,7 +1707,9 @@ declare namespace FirebaseFirestore { * @param snapshot The snapshot of the document to end before. * @return The created Query. */ - endBefore(snapshot: DocumentSnapshot): Query; + endBefore( + snapshot: DocumentSnapshot + ): Query; /** * Creates and returns a new Query that ends before the provided fields @@ -1714,7 +1720,7 @@ declare namespace FirebaseFirestore { * of the query's order by. * @return The created Query. */ - endBefore(...fieldValues: unknown[]): Query; + endBefore(...fieldValues: any[]): Query; /** * Creates and returns a new Query that ends at the provided document @@ -1725,7 +1731,9 @@ declare namespace FirebaseFirestore { * @param snapshot The snapshot of the document to end at. * @return The created Query. */ - endAt(snapshot: DocumentSnapshot): Query; + endAt( + snapshot: DocumentSnapshot + ): Query; /** * Creates and returns a new Query that ends at the provided fields @@ -1796,7 +1804,7 @@ declare namespace FirebaseFirestore { * @param other The `Query` to compare against. * @return true if this `Query` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: Query): boolean; /** * Applies a custom data converter to this Query, allowing you to use your @@ -1873,7 +1881,7 @@ declare namespace FirebaseFirestore { * @param other The `QuerySnapshot` to compare against. * @return true if this `QuerySnapshot` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: QuerySnapshot): boolean; } /** @@ -1917,7 +1925,7 @@ declare namespace FirebaseFirestore { * @param other The `DocumentChange` to compare against. * @return true if this `DocumentChange` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: DocumentChange): boolean; } /** @@ -1938,7 +1946,7 @@ declare namespace FirebaseFirestore { * A reference to the containing Document if this is a subcollection, else * null. */ - readonly parent: DocumentReference | null; + readonly parent: DocumentReference | null; /** * A string representing the path of the referenced collection (relative @@ -1999,7 +2007,7 @@ declare namespace FirebaseFirestore { * @param other The `CollectionReference` to compare against. * @return true if this `CollectionReference` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: CollectionReference): boolean; /** * Applies a custom data converter to this CollectionReference, allowing you @@ -2012,9 +2020,9 @@ declare namespace FirebaseFirestore { * `null` removes the current converter. * @return A CollectionReference that uses the provided converter. */ - withConverter( + withConverter( converter: FirestoreDataConverter - ): CollectionReference; + ): CollectionReference; withConverter(converter: null): CollectionReference; } @@ -2184,7 +2192,9 @@ declare namespace FirebaseFirestore { * * @return A promise that will be resolved with the results of the query. */ - get(): Promise>; + get(): Promise< + AggregateQuerySnapshot + >; /** * Compares this object with the given object for equality. @@ -2198,7 +2208,9 @@ declare namespace FirebaseFirestore { * @return `true` if this object is "equal" to the given object, as * defined above, or `false` otherwise. */ - isEqual(other: unknown): boolean; + isEqual( + other: AggregateQuery + ): boolean; } /** @@ -2245,7 +2257,13 @@ declare namespace FirebaseFirestore { * @return `true` if this object is "equal" to the given object, as * defined above, or `false` otherwise. */ - isEqual(other: unknown): boolean; + isEqual( + other: AggregateQuerySnapshot< + AggregateSpecType, + AppModelType, + DbModelType + > + ): boolean; } /** @@ -2324,7 +2342,7 @@ declare namespace FirebaseFirestore { * @param other The `FieldValue` to compare against. * @return true if this `FieldValue` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: FieldValue): boolean; } /** @@ -2353,7 +2371,7 @@ declare namespace FirebaseFirestore { * @param other The `FieldPath` to compare against. * @return true if this `FieldPath` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: FieldPath): boolean; } /** @@ -2438,7 +2456,7 @@ declare namespace FirebaseFirestore { * @param other The `Timestamp` to compare against. * @return 'true' if this `Timestamp` is equal to the provided one. */ - isEqual(other: unknown): boolean; + isEqual(other: Timestamp): boolean; /** * Converts this object to a primitive `string`, which allows `Timestamp` objects to be compared @@ -2463,7 +2481,9 @@ declare namespace FirebaseFirestore { * @param documentSnapshot A `DocumentSnapshot` to add. * @returns This instance. */ - add(documentSnapshot: DocumentSnapshot): BundleBuilder; + add( + documentSnapshot: DocumentSnapshot + ): BundleBuilder; /** * Adds a Firestore `QuerySnapshot` to the bundle. Both the documents in the query results and @@ -2473,7 +2493,10 @@ declare namespace FirebaseFirestore { * @param querySnapshot A `QuerySnapshot` to add to the bundle. * @returns This instance. */ - add(queryName: string, querySnapshot: QuerySnapshot): BundleBuilder; + add( + queryName: string, + querySnapshot: QuerySnapshot + ): BundleBuilder; /** * Builds the bundle and returns the result as a `Buffer` instance. @@ -2560,7 +2583,7 @@ declare namespace FirebaseFirestore { static where( fieldPath: string | FieldPath, opStr: WhereFilterOp, - value: unknown + value: any ): Filter; /**