Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Fix the UpdateData incorrect parameter type issue #1887

Merged
merged 28 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
18233c5
WIP
tom-andersen Jul 17, 2023
d1d311f
WIP
tom-andersen Jul 18, 2023
296649d
Add parameter types.
tom-andersen Aug 23, 2023
ccffd19
Fix
tom-andersen Aug 23, 2023
3603e2e
Fix
tom-andersen Aug 23, 2023
f792c79
Fix
tom-andersen Aug 24, 2023
02a10db
Fix
tom-andersen Aug 24, 2023
1a7689d
Merge branch 'main' into tomandersen/updatedocParameterTypes
tom-andersen Aug 29, 2023
5fc2c58
Add Test
tom-andersen Aug 29, 2023
09b5db0
Merge branch 'main' into tomandersen/updatedocParameterTypes
tom-andersen Sep 1, 2023
46ae472
Compile with TS 5.1.3
tom-andersen Sep 13, 2023
0bd0eca
Fix
tom-andersen Sep 13, 2023
cfe3024
Fix
tom-andersen Sep 13, 2023
76ed46f
Fix running of tests.
tom-andersen Sep 14, 2023
4eefcf1
PR Feedback and fix types.
tom-andersen Sep 15, 2023
733fd40
PR Feedback
tom-andersen Sep 21, 2023
342ee95
PR feedback and remove pinning of DbModelType
tom-andersen Sep 21, 2023
0a70f14
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 21, 2023
a76cb0c
Merge branch 'main' into tomandersen/updatedocParameterTypes
tom-andersen Sep 21, 2023
7c04471
Last PR Review Changes
tom-andersen Sep 22, 2023
f6a972b
PR Review Changes
tom-andersen Sep 25, 2023
80f5309
Remove unused import
tom-andersen Sep 25, 2023
9cada8f
Missed review comment
tom-andersen Sep 25, 2023
2ee882e
Review changes
tom-andersen Sep 25, 2023
2c34f97
Make compile with new version of NodeJS
tom-andersen Sep 25, 2023
c0c8519
Merge branch 'main' into tomandersen/updatedocParameterTypes
tom-andersen Sep 26, 2023
cdd9382
Pin types, as before.
tom-andersen Sep 26, 2023
1c15b65
Merge branch 'main' into tomandersen/updatedocParameterTypes
sofisl Sep 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
1 change: 0 additions & 1 deletion dev/src/bulk-writer.ts
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
MAX_RETRY_ATTEMPTS,
} from './backoff';
import {RateLimiter} from './rate-limiter';
import {DocumentReference} from './reference';
import {Timestamp} from './timestamp';
import {
Deferred,
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
2 changes: 1 addition & 1 deletion dev/src/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import * as firestore from '@google-cloud/firestore';
* or {@link Filter#and} and can then be passed to {@link Query#where}
* to create a new {@link Query} instance that also contains this `Filter`.
*/
export abstract class Filter extends firestore.Filter {
export abstract class Filter {
/**
* Creates and returns a new [Filter]{@link Filter}, which can be
* applied to [Query.where()]{@link Query#where}, [Filter.or()]{@link Filter#or},
Expand Down
85 changes: 42 additions & 43 deletions dev/src/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,7 @@
* });
* ```
*/
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 @@
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 @@
* @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 @@
/** 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 @@
* @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 @@
*/
static forCollectionGroupQuery<
AppModelType,
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
DbModelType extends firestore.DocumentData = firestore.DocumentData
DbModelType extends firestore.DocumentData
>(
collectionId: string,
converter: firestore.FirestoreDataConverter<
Expand Down Expand Up @@ -1374,7 +1372,7 @@
return this.fieldOrders.length > 0;
}

isEqual(other: unknown): boolean {
isEqual(other: QueryOptions<AppModelType, DbModelType>): boolean {
if (this === other) {
return true;
}
Expand Down Expand Up @@ -1517,7 +1515,7 @@
* "in", "not-in", and "array-contains-any".
* @param {*} value The value to which to compare the field for inclusion in
* a query.
* @returns {Query} The created Query.

Check warning on line 1518 in dev/src/reference.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
*
* @example
* ```
Expand All @@ -1531,9 +1529,9 @@
* ```
*/
where(
fieldPath: string | firestore.FieldPath,
fieldPath: string | FieldPath,
opStr: firestore.WhereFilterOp,
value: unknown
value: any
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
): Query<AppModelType, DbModelType>;

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

where(
fieldPathOrFilter: string | firestore.FieldPath | firestore.Filter,
fieldPathOrFilter: string | FieldPath | Filter,
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
opStr?: firestore.WhereFilterOp,
value?: unknown
): Query<AppModelType, DbModelType> {
let filter: firestore.Filter;
dconeybe marked this conversation as resolved.
Show resolved Hide resolved

if (fieldPathOrFilter instanceof firestore.Filter) {
if (fieldPathOrFilter instanceof Filter) {
filter = fieldPathOrFilter;
} else {
filter = Filter.where(fieldPathOrFilter, opStr!, value);
Expand Down Expand Up @@ -2096,7 +2094,9 @@
* ```
*/
startAt(
...fieldValuesOrDocumentSnapshot: unknown[]
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.startAt',
Expand Down Expand Up @@ -2140,7 +2140,9 @@
* ```
*/
startAfter(
...fieldValuesOrDocumentSnapshot: Array<unknown>
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.startAfter',
Expand Down Expand Up @@ -2183,7 +2185,9 @@
* ```
*/
endBefore(
...fieldValuesOrDocumentSnapshot: Array<unknown>
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.endBefore',
Expand Down Expand Up @@ -2226,7 +2230,9 @@
* ```
*/
endAt(
...fieldValuesOrDocumentSnapshot: Array<unknown>
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<unknown> | unknown
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
'Query.endAt',
Expand Down Expand Up @@ -2683,9 +2689,7 @@
* ```
*/
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 @@
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 @@
* 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 @@
) {}

/** 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 @@
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 @@
* @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 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 @@
) {}

/** 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
Loading
Loading