Skip to content

Commit

Permalink
feat!: upgrade Typescript to v4.1.5 and add converter and typing upgr…
Browse files Browse the repository at this point in the history 
…ades (#1632)
  • Loading branch information
Brian Chen committed Nov 23, 2021
1 parent a4dbac5 commit c293955
Show file tree
Hide file tree
Showing 16 changed files with 1,132 additions and 160 deletions.
2 changes: 1 addition & 1 deletion dev/conformance/runner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ function runTest(spec: ConformanceProto) {
};

return createInstance(overrides).then(() => {
return new Promise((resolve, reject) => {
return new Promise<void>((resolve, reject) => {
const unlisten = watchQuery().onSnapshot(
actualSnap => {
const expectedSnapshot = expectedSnapshots.shift();
Expand Down
19 changes: 13 additions & 6 deletions dev/src/bulk-writer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ export class BulkWriter {
*/
create<T>(
documentRef: firestore.DocumentReference<T>,
data: T
data: firestore.WithFieldValue<T>
): Promise<WriteResult> {
this._verifyNotClosed();
return this._enqueue(documentRef, 'create', bulkCommitBatch =>
Expand Down Expand Up @@ -672,13 +672,20 @@ export class BulkWriter {
*/
set<T>(
documentRef: firestore.DocumentReference<T>,
data: T | Partial<T>,
data: firestore.PartialWithFieldValue<T>,
options?: firestore.SetOptions
): Promise<WriteResult> {
this._verifyNotClosed();
return this._enqueue(documentRef, 'set', bulkCommitBatch =>
bulkCommitBatch.set(documentRef, data, options)
);
return this._enqueue(documentRef, 'set', bulkCommitBatch => {
if (options) {
return bulkCommitBatch.set(documentRef, data, options);
} else {
return bulkCommitBatch.set(
documentRef,
data as firestore.WithFieldValue<T>
);
}
});
}

/**
Expand Down Expand Up @@ -726,7 +733,7 @@ export class BulkWriter {
*/
update<T>(
documentRef: firestore.DocumentReference<T>,
dataOrField: firestore.UpdateData | string | FieldPath,
dataOrField: firestore.UpdateData<T> | string | FieldPath,
...preconditionOrValues: Array<
{lastUpdateTime?: Timestamp} | unknown | string | FieldPath
>
Expand Down
2 changes: 1 addition & 1 deletion dev/src/document-change.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export type DocumentChangeType = 'added' | 'removed' | 'modified';
* @class DocumentChange
*/
export class DocumentChange<T = firestore.DocumentData>
implements firestore.DocumentChange
implements firestore.DocumentChange<T>
{
private readonly _type: DocumentChangeType;
private readonly _document: QueryDocumentSnapshot<T>;
Expand Down
28 changes: 17 additions & 11 deletions dev/src/reference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ export class DocumentReference<T = firestore.DocumentData>
*
* @param _firestore The Firestore Database client.
* @param _path The Path of this reference.
* @param _converter The converter to use when serializing data.
*/
constructor(
private readonly _firestore: Firestore,
Expand Down Expand Up @@ -372,7 +373,7 @@ export class DocumentReference<T = firestore.DocumentData>
* });
* ```
*/
create(data: T): Promise<WriteResult> {
create(data: firestore.WithFieldValue<T>): Promise<WriteResult> {
const writeBatch = new WriteBatch(this._firestore);
return writeBatch
.create(this, data)
Expand Down Expand Up @@ -413,8 +414,11 @@ export class DocumentReference<T = firestore.DocumentData>
.then(([writeResult]) => writeResult);
}

set(data: Partial<T>, options: firestore.SetOptions): Promise<WriteResult>;
set(data: T): Promise<WriteResult>;
set(
data: firestore.PartialWithFieldValue<T>,
options: firestore.SetOptions
): Promise<WriteResult>;
set(data: firestore.WithFieldValue<T>): Promise<WriteResult>;
/**
* Writes to the document referred to by this DocumentReference. If the
* document does not yet exist, it will be created. If you pass
Expand All @@ -441,14 +445,16 @@ export class DocumentReference<T = firestore.DocumentData>
* ```
*/
set(
data: T | Partial<T>,
data: firestore.PartialWithFieldValue<T>,
options?: firestore.SetOptions
): Promise<WriteResult> {
const writeBatch = new WriteBatch(this._firestore);
return writeBatch
.set(this, data, options)
.commit()
.then(([writeResult]) => writeResult);
let writeBatch = new WriteBatch(this._firestore);
if (options) {
writeBatch = writeBatch.set(this, data, options);
} else {
writeBatch = writeBatch.set(this, data as firestore.WithFieldValue<T>);
}
return writeBatch.commit().then(([writeResult]) => writeResult);
}

/**
Expand Down Expand Up @@ -483,7 +489,7 @@ export class DocumentReference<T = firestore.DocumentData>
* ```
*/
update(
dataOrField: firestore.UpdateData | string | firestore.FieldPath,
dataOrField: firestore.UpdateData<T> | string | firestore.FieldPath,
...preconditionOrValues: Array<
unknown | string | firestore.FieldPath | firestore.Precondition
>
Expand Down Expand Up @@ -2691,7 +2697,7 @@ export class CollectionReference<T = firestore.DocumentData>
* });
* ```
*/
add(data: T): Promise<DocumentReference<T>> {
add(data: firestore.WithFieldValue<T>): Promise<DocumentReference<T>> {
const firestoreData = this._queryOptions.converter.toFirestore(data);
validateDocumentData(
'data',
Expand Down
22 changes: 16 additions & 6 deletions dev/src/transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,17 +218,23 @@ export class Transaction implements firestore.Transaction {
* });
* ```
*/
create<T>(documentRef: firestore.DocumentReference<T>, data: T): Transaction {
create<T>(
documentRef: firestore.DocumentReference<T>,
data: firestore.WithFieldValue<T>
): Transaction {
this._writeBatch.create(documentRef, data);
return this;
}

set<T>(
documentRef: firestore.DocumentReference<T>,
data: Partial<T>,
data: firestore.PartialWithFieldValue<T>,
options: firestore.SetOptions
): Transaction;
set<T>(documentRef: firestore.DocumentReference<T>, data: T): Transaction;
set<T>(
documentRef: firestore.DocumentReference<T>,
data: firestore.WithFieldValue<T>
): Transaction;
/**
* Writes to the document referred to by the provided
* [DocumentReference]{@link DocumentReference}. If the document
Expand Down Expand Up @@ -260,10 +266,14 @@ export class Transaction implements firestore.Transaction {
*/
set<T>(
documentRef: firestore.DocumentReference<T>,
data: T | Partial<T>,
data: firestore.PartialWithFieldValue<T>,
options?: firestore.SetOptions
): Transaction {
this._writeBatch.set(documentRef, data, options);
if (options) {
this._writeBatch.set(documentRef, data, options);
} else {
this._writeBatch.set(documentRef, data as firestore.WithFieldValue<T>);
}
return this;
}

Expand Down Expand Up @@ -309,7 +319,7 @@ export class Transaction implements firestore.Transaction {
*/
update<T>(
documentRef: firestore.DocumentReference<T>,
dataOrField: firestore.UpdateData | string | firestore.FieldPath,
dataOrField: firestore.UpdateData<T> | string | firestore.FieldPath,
...preconditionOrValues: Array<
firestore.Precondition | unknown | string | firestore.FieldPath
>
Expand Down
8 changes: 6 additions & 2 deletions dev/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
* limitations under the License.
*/

import {FirestoreDataConverter, DocumentData} from '@google-cloud/firestore';
import {
FirestoreDataConverter,
DocumentData,
WithFieldValue,
} from '@google-cloud/firestore';

import {CallOptions} from 'google-gax';
import {Duplex} from 'stream';
Expand Down Expand Up @@ -114,7 +118,7 @@ export type RBTree = any;
* @internal
*/
const defaultConverterObj: FirestoreDataConverter<DocumentData> = {
toFirestore(modelObject: DocumentData): DocumentData {
toFirestore(modelObject: WithFieldValue<DocumentData>): DocumentData {
return modelObject;
},
fromFirestore(snapshot: QueryDocumentSnapshot): DocumentData {
Expand Down
10 changes: 5 additions & 5 deletions dev/src/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ import * as gapicConfig from './v1/firestore_client_config.json';
*/
export class Deferred<R> {
promise: Promise<R>;
resolve: (value?: R | Promise<R>) => void = () => {};
reject: (reason?: Error) => void = () => {};
resolve: (value: R | Promise<R>) => void = () => {};
reject: (reason: Error) => void = () => {};

constructor() {
this.promise = new Promise(
this.promise = new Promise<R>(
(
resolve: (value?: R | Promise<R>) => void,
reject: (reason?: Error) => void
resolve: (value: R | Promise<R>) => void,
reject: (reason: Error) => void
) => {
this.resolve = resolve;
this.reject = reject;
Expand Down
60 changes: 23 additions & 37 deletions dev/src/write-batch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,14 @@ export class WriteBatch implements firestore.WriteBatch {
* });
* ```
*/
create<T>(documentRef: firestore.DocumentReference<T>, data: T): WriteBatch {
create<T>(
documentRef: firestore.DocumentReference<T>,
data: firestore.WithFieldValue<T>
): WriteBatch {
const ref = validateDocumentReference('documentRef', documentRef);
const firestoreData = ref._converter.toFirestore(data);
const firestoreData = ref._converter.toFirestore(
data as firestore.WithFieldValue<T>
);
validateDocumentData(
'data',
firestoreData,
Expand Down Expand Up @@ -274,14 +279,12 @@ export class WriteBatch implements firestore.WriteBatch {

set<T>(
documentRef: firestore.DocumentReference<T>,
data: Partial<T>,
data: firestore.PartialWithFieldValue<T>,
options: firestore.SetOptions
): WriteBatch;
set<T>(documentRef: firestore.DocumentReference<T>, data: T): WriteBatch;
set<T>(
documentRef: firestore.DocumentReference<T>,
data: T | Partial<T>,
options?: firestore.SetOptions
data: firestore.WithFieldValue<T>
): WriteBatch;
/**
* Write to the document referred to by the provided
Expand Down Expand Up @@ -316,12 +319,12 @@ export class WriteBatch implements firestore.WriteBatch {
*/
set<T>(
documentRef: firestore.DocumentReference<T>,
data: T | Partial<T>,
data: firestore.PartialWithFieldValue<T>,
options?: firestore.SetOptions
): WriteBatch {
validateSetOptions('options', options, {optional: true});
const mergeLeaves = options && options.merge === true;
const mergePaths = options && options.mergeFields;
const mergeLeaves = options && 'merge' in options && options.merge;
const mergePaths = options && 'mergeFields' in options;
const ref = validateDocumentReference('documentRef', documentRef);
let firestoreData: firestore.DocumentData;
if (mergeLeaves || mergePaths) {
Expand Down Expand Up @@ -415,7 +418,7 @@ export class WriteBatch implements firestore.WriteBatch {
*/
update<T = firestore.DocumentData>(
documentRef: firestore.DocumentReference<T>,
dataOrField: firestore.UpdateData | string | firestore.FieldPath,
dataOrField: firestore.UpdateData<T> | string | firestore.FieldPath,
...preconditionOrValues: Array<
| {lastUpdateTime?: firestore.Timestamp}
| unknown
Expand Down Expand Up @@ -480,15 +483,16 @@ export class WriteBatch implements firestore.WriteBatch {
// eslint-disable-next-line prefer-rest-params
validateMaxNumberOfArguments('update', arguments, 3);

const data = dataOrField as firestore.UpdateData;
Object.entries(data).forEach(([key, value]) => {
// Skip `undefined` values (can be hit if `ignoreUndefinedProperties`
// is set)
if (value !== undefined) {
validateFieldPath(key, key);
updateMap.set(FieldPath.fromArgument(key), value);
Object.entries(dataOrField as firestore.UpdateData<T>).forEach(
([key, value]) => {
// Skip `undefined` values (can be hit if `ignoreUndefinedProperties`
// is set)
if (value !== undefined) {
validateFieldPath(key, key);
updateMap.set(FieldPath.fromArgument(key), value);
}
}
});
);

if (preconditionOrValues.length > 0) {
validateUpdatePrecondition(
Expand Down Expand Up @@ -771,27 +775,9 @@ export function validateSetOptions(
);
}

const setOptions = value as {[k: string]: unknown};

if ('merge' in setOptions && typeof setOptions.merge !== 'boolean') {
throw new Error(
`${invalidArgumentMessage(
arg,
'set() options argument'
)} "merge" is not a boolean.`
);
}
const setOptions = value as {mergeFields: Array<string | FieldPath>};

if ('mergeFields' in setOptions) {
if (!Array.isArray(setOptions.mergeFields)) {
throw new Error(
`${invalidArgumentMessage(
arg,
'set() options argument'
)} "mergeFields" is not an array.`
);
}

for (let i = 0; i < setOptions.mergeFields.length; ++i) {
try {
validateFieldPath(i, setOptions.mergeFields[i]);
Expand Down
Loading

2 comments on commit c293955

@mo22
Copy link

@mo22 mo22 commented on c293955 Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, not absolutely sure but it looks to me that this change breaks nested field updates in the likes of:

docRef.update({
  [`some.nested.field`]: 'some value'
});

I think this is because the data argument to update() was previously not templated.

@thebrianchen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mo22 I was able to perform the same updates against the latest version successfully. If you're unable to, could you please file an issue in this repo with steps for a reproduction? Thanks!

Please sign in to comment.