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 19 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
41 changes: 20 additions & 21 deletions 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 @@
MAX_RETRY_ATTEMPTS,
} from './backoff';
import {RateLimiter} from './rate-limiter';
import {DocumentReference} from './reference';
import {Timestamp} from './timestamp';
import {
Deferred,
Expand Down Expand Up @@ -337,7 +336,7 @@
readonly message: string,

/** The document reference the operation was performed on. */
readonly documentRef: firestore.DocumentReference<unknown>,
readonly documentRef: firestore.DocumentReference<any, any>,

Check warning on line 339 in dev/src/bulk-writer.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 339 in dev/src/bulk-writer.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

/** The type of operation performed. */
readonly operationType: 'create' | 'set' | 'update' | 'delete',
Expand Down Expand Up @@ -576,9 +575,9 @@
* });
* ```
*/
create<T>(
documentRef: firestore.DocumentReference<T>,
data: firestore.WithFieldValue<T>
create<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: firestore.DocumentReference<AppModelType, DbModelType>,
data: firestore.WithFieldValue<AppModelType>
): Promise<WriteResult> {
this._verifyNotClosed();
return this._enqueue(documentRef, 'create', bulkCommitBatch =>
Expand Down Expand Up @@ -616,8 +615,8 @@
* });
* ```
*/
delete<T>(
documentRef: firestore.DocumentReference<T>,
delete<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: firestore.DocumentReference<AppModelType, DbModelType>,
precondition?: firestore.Precondition
): Promise<WriteResult> {
this._verifyNotClosed();
Expand All @@ -626,14 +625,14 @@
);
}

set<T>(
documentRef: firestore.DocumentReference<T>,
data: Partial<T>,
set<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: firestore.DocumentReference<AppModelType, DbModelType>,
data: Partial<AppModelType>,
options: firestore.SetOptions
): Promise<WriteResult>;
set<T>(
documentRef: firestore.DocumentReference<T>,
data: T
set<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: firestore.DocumentReference<AppModelType, DbModelType>,
data: AppModelType
): Promise<WriteResult>;
/**
* Write to the document referred to by the provided
Expand Down Expand Up @@ -675,9 +674,9 @@
* });
* ```
*/
set<T>(
documentRef: firestore.DocumentReference<T>,
data: firestore.PartialWithFieldValue<T>,
set<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: firestore.DocumentReference<AppModelType, DbModelType>,
data: firestore.PartialWithFieldValue<AppModelType>,
options?: firestore.SetOptions
): Promise<WriteResult> {
this._verifyNotClosed();
Expand All @@ -687,7 +686,7 @@
} else {
return bulkCommitBatch.set(
documentRef,
data as firestore.WithFieldValue<T>
data as firestore.WithFieldValue<AppModelType>
);
}
});
Expand Down Expand Up @@ -737,9 +736,9 @@
* });
* ```
*/
update<T>(
documentRef: firestore.DocumentReference<T>,
dataOrField: firestore.UpdateData<T> | string | FieldPath,
update<AppModelType, DbModelType extends firestore.DocumentData>(
documentRef: firestore.DocumentReference<AppModelType, DbModelType>,
dataOrField: firestore.UpdateData<DbModelType> | string | FieldPath,
...preconditionOrValues: Array<
{lastUpdateTime?: Timestamp} | unknown | string | FieldPath
>
Expand Down Expand Up @@ -783,7 +782,7 @@
*/
onWriteResult(
successCallback: (
documentRef: firestore.DocumentReference<unknown>,
documentRef: firestore.DocumentReference<any, any>,

Check warning on line 785 in dev/src/bulk-writer.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 785 in dev/src/bulk-writer.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
result: WriteResult
) => void
): void {
Expand Down
39 changes: 24 additions & 15 deletions dev/src/collection-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,20 @@ import {compareArrays} from './order';
*
* @class CollectionGroup
*/
export class CollectionGroup<T = firestore.DocumentData>
extends Query<T>
implements firestore.CollectionGroup<T>
export class CollectionGroup<
AppModelType = firestore.DocumentData,
DbModelType extends firestore.DocumentData = firestore.DocumentData
>
extends Query<AppModelType, DbModelType>
implements firestore.CollectionGroup<AppModelType, DbModelType>
{
/** @private */
constructor(
firestore: Firestore,
collectionId: string,
converter: firestore.FirestoreDataConverter<T> | undefined
converter:
| firestore.FirestoreDataConverter<AppModelType, DbModelType>
| undefined
) {
super(
firestore,
Expand Down Expand Up @@ -74,7 +79,7 @@ export class CollectionGroup<T = firestore.DocumentData>
*/
async *getPartitions(
desiredPartitionCount: number
): AsyncIterable<QueryPartition<T>> {
): AsyncIterable<QueryPartition<AppModelType, DbModelType>> {
validateInteger('desiredPartitionCount', desiredPartitionCount, {
minValue: 1,
});
Expand Down Expand Up @@ -141,7 +146,8 @@ export class CollectionGroup<T = firestore.DocumentData>
* Applies a custom data converter to this `CollectionGroup`, allowing you
* to use your own custom model objects with Firestore. When you call get()
* on the returned `CollectionGroup`, the provided converter will convert
* between Firestore data and your custom type U.
* between Firestore data of type `NewDbModelType` and your custom type
* `NewAppModelType`.
*
* Using the converter allows you to specify generic type arguments when
* storing and retrieving objects from Firestore.
Expand Down Expand Up @@ -185,17 +191,20 @@ export class CollectionGroup<T = firestore.DocumentData>
* ```
* @param {FirestoreDataConverter | null} converter Converts objects to and
* from Firestore. Passing in `null` removes the current converter.
* @return {CollectionGroup} A `CollectionGroup<U>` that uses the provided
* @return {CollectionGroup} A `CollectionGroup` that uses the provided
* converter.
*/
withConverter(converter: null): CollectionGroup<firestore.DocumentData>;
withConverter<U>(
converter: firestore.FirestoreDataConverter<U>
): CollectionGroup<U>;
withConverter<U>(
converter: firestore.FirestoreDataConverter<U> | null
): CollectionGroup<U> {
return new CollectionGroup<U>(
withConverter(converter: null): CollectionGroup;
withConverter<NewAppModelType, NewDbModelType extends firestore.DocumentData>(
converter: firestore.FirestoreDataConverter<NewAppModelType, NewDbModelType>
): CollectionGroup<NewAppModelType, NewDbModelType>;
withConverter<NewAppModelType, NewDbModelType extends firestore.DocumentData>(
converter: firestore.FirestoreDataConverter<
NewAppModelType,
NewDbModelType
> | null
): CollectionGroup<NewAppModelType, NewDbModelType> {
return new CollectionGroup<NewAppModelType, NewDbModelType>(
this.firestore,
this._queryOptions.collectionId,
converter ?? defaultConverter()
Expand Down
14 changes: 8 additions & 6 deletions dev/src/document-change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ export type DocumentChangeType = 'added' | 'removed' | 'modified';
*
* @class DocumentChange
*/
export class DocumentChange<T = firestore.DocumentData>
implements firestore.DocumentChange<T>
export class DocumentChange<
AppModelType = firestore.DocumentData,
DbModelType extends firestore.DocumentData = firestore.DocumentData
> implements firestore.DocumentChange<AppModelType, DbModelType>
{
private readonly _type: DocumentChangeType;
private readonly _document: QueryDocumentSnapshot<T>;
private readonly _document: QueryDocumentSnapshot<AppModelType, DbModelType>;
private readonly _oldIndex: number;
private readonly _newIndex: number;

Expand All @@ -46,7 +48,7 @@ export class DocumentChange<T = firestore.DocumentData>
*/
constructor(
type: DocumentChangeType,
document: QueryDocumentSnapshot<T>,
document: QueryDocumentSnapshot<AppModelType, DbModelType>,
oldIndex: number,
newIndex: number
) {
Expand Down Expand Up @@ -103,7 +105,7 @@ export class DocumentChange<T = firestore.DocumentData>
* unsubscribe();
* ```
*/
get doc(): QueryDocumentSnapshot<T> {
get doc(): QueryDocumentSnapshot<AppModelType, DbModelType> {
return this._document;
}

Expand Down Expand Up @@ -181,7 +183,7 @@ export class DocumentChange<T = firestore.DocumentData>
* @param {*} other The value to compare against.
* @return true if this `DocumentChange` is equal to the provided value.
*/
isEqual(other: firestore.DocumentChange<T>): boolean {
isEqual(other: firestore.DocumentChange<AppModelType, DbModelType>): boolean {
if (this === other) {
return true;
}
Expand Down
16 changes: 11 additions & 5 deletions dev/src/document-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import api = google.firestore.v1;
* @private
* @internal
*/
export class DocumentReader<T> {
export class DocumentReader<
AppModelType,
DbModelType extends DocumentData = DocumentData
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
> {
/** An optional field mask to apply to this read. */
fieldMask?: FieldPath[];
/** An optional transaction ID to use for this read. */
Expand All @@ -49,7 +52,7 @@ export class DocumentReader<T> {
*/
constructor(
private firestore: Firestore,
private allDocuments: Array<DocumentReference<T>>
private allDocuments: Array<DocumentReference<AppModelType, DbModelType>>
) {
for (const docRef of this.allDocuments) {
this.outstandingDocuments.add(docRef.formattedName);
Expand All @@ -61,20 +64,23 @@ export class DocumentReader<T> {
*
* @param requestTag A unique client-assigned identifier for this request.
*/
async get(requestTag: string): Promise<Array<DocumentSnapshot<T>>> {
async get(
requestTag: string
): Promise<Array<DocumentSnapshot<AppModelType, DbModelType>>> {
await this.fetchDocuments(requestTag);

// BatchGetDocuments doesn't preserve document order. We use the request
// order to sort the resulting documents.
const orderedDocuments: Array<DocumentSnapshot<T>> = [];
const orderedDocuments: Array<DocumentSnapshot<AppModelType, DbModelType>> =
[];

for (const docRef of this.allDocuments) {
const document = this.retrievedDocuments.get(docRef.formattedName);
if (document !== undefined) {
// Recreate the DocumentSnapshot with the DocumentReference
// containing the original converter.
const finalDoc = new DocumentSnapshotBuilder(
docRef as DocumentReference<T>
docRef as DocumentReference<AppModelType, DbModelType>
);
finalDoc.fieldsProto = document._fieldsProto;
finalDoc.readTime = document.readTime;
Expand Down
Loading