Skip to content

Commit

Permalink
PR Feedback and fix types.
Browse files Browse the repository at this point in the history
  • Loading branch information
tom-andersen committed Sep 15, 2023
1 parent 76ed46f commit 4eefcf1
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 100 deletions.
2 changes: 1 addition & 1 deletion dev/conformance/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion dev/src/document-change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AppModelType, DbModelType>): boolean {
if (this === other) {
return true;
}
Expand Down
23 changes: 12 additions & 11 deletions dev/src/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -158,14 +157,11 @@ export class DocumentSnapshot<
* @return The created DocumentSnapshot.
*/
static fromObject<AppModelType, DbModelType extends firestore.DocumentData>(
ref: firestore.DocumentReference<AppModelType, DbModelType>,
ref: DocumentReference<AppModelType, DbModelType>,
obj: firestore.DocumentData
): DocumentSnapshot<AppModelType, DbModelType> {
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.
Expand Down Expand Up @@ -440,7 +436,7 @@ export class DocumentSnapshot<
this.readTime,
this.createTime!,
this.updateTime!
) as firestore.QueryDocumentSnapshot
)
);
} else {
const obj: firestore.DocumentData = {};
Expand Down Expand Up @@ -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<AppModelType, DbModelType>
): boolean {
// Since the read time is different on every document read, we explicitly
// ignore all document metadata in this comparison.
return (
Expand Down Expand Up @@ -986,7 +984,10 @@ export class DocumentTransform<
updateMap.set(new FieldPath(prop), obj[prop]);
}

return DocumentTransform.fromUpdateMap(ref, updateMap);
return DocumentTransform.fromUpdateMap<AppModelType, DbModelType>(
ref,
updateMap
);
}

/**
Expand Down
81 changes: 40 additions & 41 deletions dev/src/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,7 @@ export class DocumentReference<
* });
* ```
*/
listCollections(): Promise<
Array<CollectionReference<firestore.DocumentData>>
> {
listCollections(): Promise<Array<CollectionReference>> {
const tag = requestTag();
return this.firestore.initializeIfNeeded(tag).then(() => {
const request: api.IListCollectionIdsRequest = {
Expand All @@ -353,9 +351,7 @@ export class DocumentReference<
tag
)
.then(collectionIds => {
const collections: Array<
CollectionReference<firestore.DocumentData>
> = [];
const collections: Array<CollectionReference> = [];

// We can just sort this list using the default comparator since it
// will only contain collection ids.
Expand Down Expand Up @@ -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<AppModelType, DbModelType>
): boolean {
return (
this === other ||
(other instanceof DocumentReference &&
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<AppModelType, DbModelType>): boolean {
// Since the read time is different on every query read, we explicitly
// ignore all metadata in this comparison.

Expand Down Expand Up @@ -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<
Expand Down Expand Up @@ -1374,7 +1372,7 @@ export class QueryOptions<
return this.fieldOrders.length > 0;
}

isEqual(other: unknown): boolean {
isEqual(other: QueryOptions<AppModelType, DbModelType>): boolean {
if (this === other) {
return true;
}
Expand Down Expand Up @@ -1531,9 +1529,9 @@ export class Query<
* ```
*/
where(
fieldPath: string | firestore.FieldPath,
fieldPath: string | FieldPath,
opStr: firestore.WhereFilterOp,
value: unknown
value: any
): Query<AppModelType, DbModelType>;

/**
Expand All @@ -1558,7 +1556,7 @@ export class Query<
* });
* ```
*/
where(filter: firestore.Filter): Query<AppModelType, DbModelType>;
where(filter: Filter): Query<AppModelType, DbModelType>;

where(
fieldPathOrFilter: string | FieldPath | Filter,
Expand Down Expand Up @@ -2096,7 +2094,9 @@ export class Query<
* ```
*/
startAt(
...fieldValuesOrDocumentSnapshot: unknown[]
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.startAt',
Expand Down Expand Up @@ -2140,7 +2140,9 @@ export class Query<
* ```
*/
startAfter(
...fieldValuesOrDocumentSnapshot: Array<unknown>
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.startAfter',
Expand Down Expand Up @@ -2183,7 +2185,9 @@ export class Query<
* ```
*/
endBefore(
...fieldValuesOrDocumentSnapshot: Array<unknown>
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.endBefore',
Expand Down Expand Up @@ -2226,7 +2230,9 @@ export class Query<
* ```
*/
endAt(
...fieldValuesOrDocumentSnapshot: Array<unknown>
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.endAt',
Expand Down Expand Up @@ -2683,9 +2689,7 @@ export class Query<
* ```
*/
onSnapshot(
onNext: (
snapshot: firestore.QuerySnapshot<AppModelType, DbModelType>
) => void,
onNext: (snapshot: QuerySnapshot<AppModelType, DbModelType>) => void,
onError?: (error: Error) => void
): () => void {
validateFunction('onNext', onNext);
Expand Down Expand Up @@ -2844,7 +2848,7 @@ export class CollectionReference<
constructor(
firestore: Firestore,
path: ResourcePath,
converter = defaultConverter<AppModelType, DbModelType>()
converter?: firestore.FirestoreDataConverter<AppModelType, DbModelType>
) {
super(firestore, QueryOptions.forCollectionQuery(path, converter));
}
Expand Down Expand Up @@ -2892,7 +2896,7 @@ export class CollectionReference<
* console.log(`Parent name: ${documentRef.path}`);
* ```
*/
get parent(): DocumentReference<firestore.DocumentData> | null {
get parent(): DocumentReference | null {
if (this._queryOptions.parentPath.isDocument) {
return new DocumentReference(
this.firestore,
Expand Down Expand Up @@ -3176,7 +3180,7 @@ export class AggregateQuery<
) {}

/** The query whose aggregations will be calculated by this object. */
get query(): firestore.Query<AppModelType, DbModelType> {
get query(): Query<AppModelType, DbModelType> {
return this._query;
}

Expand Down Expand Up @@ -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'));
}
Expand Down Expand Up @@ -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<AggregateSpecType>): boolean {
isEqual(
other: firestore.AggregateQuery<
AggregateSpecType,
AppModelType,
DbModelType
>
): boolean {
if (this === other) {
return true;
}
Expand All @@ -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,
Expand Down Expand Up @@ -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<AggregateSpecType, AppModelType, DbModelType> {
return this._query;
}

/** The time this snapshot was read. */
get readTime(): firestore.Timestamp {
get readTime(): Timestamp {
return this._readTime;
}

Expand Down
11 changes: 7 additions & 4 deletions dev/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class Transaction implements firestore.Transaction {
* @return {Promise<DocumentSnapshot>} A DocumentSnapshot for the read data.
*/
get<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: DocumentReference<AppModelType, DbModelType>
documentRef: firestore.DocumentReference<AppModelType, DbModelType>
): Promise<DocumentSnapshot<AppModelType, DbModelType>>;

/**
Expand Down Expand Up @@ -413,7 +413,10 @@ export class Transaction implements firestore.Transaction {
documentRef: DocumentReference<AppModelType, DbModelType>,
precondition?: firestore.Precondition
): this {
this._writeBatch.delete(documentRef, precondition);
this._writeBatch.delete<AppModelType, DbModelType>(
documentRef,
precondition
);
return this;
}

Expand Down Expand Up @@ -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<AppModelType, DbModelType>
Expand Down Expand Up @@ -621,7 +624,7 @@ export function parseGetAllArguments<
}

for (let i = 0; i < documents.length; ++i) {
validateDocumentReference(i, documents[i]);
validateDocumentReference<AppModelType, DbModelType>(i, documents[i]);
}

validateReadOptions('options', readOptions, {optional: true});
Expand Down
11 changes: 4 additions & 7 deletions dev/src/write-batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
Loading

0 comments on commit 4eefcf1

Please sign in to comment.