diff --git a/dev/conformance/runner.ts b/dev/conformance/runner.ts index 1fb08e35f..02b1a7595 100644 --- a/dev/conformance/runner.ts +++ b/dev/conformance/runner.ts @@ -432,7 +432,7 @@ function runTest(spec: ConformanceProto) { }; return createInstance(overrides).then(() => { - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const unlisten = watchQuery().onSnapshot( actualSnap => { const expectedSnapshot = expectedSnapshots.shift(); diff --git a/dev/src/bulk-writer.ts b/dev/src/bulk-writer.ts index 539c42dc6..e55bd0e90 100644 --- a/dev/src/bulk-writer.ts +++ b/dev/src/bulk-writer.ts @@ -576,7 +576,7 @@ export class BulkWriter { */ create( documentRef: firestore.DocumentReference, - data: T + data: firestore.WithFieldValue ): Promise { this._verifyNotClosed(); return this._enqueue(documentRef, 'create', bulkCommitBatch => @@ -672,13 +672,20 @@ export class BulkWriter { */ set( documentRef: firestore.DocumentReference, - data: T | Partial, + data: firestore.PartialWithFieldValue, options?: firestore.SetOptions ): Promise { 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 + ); + } + }); } /** @@ -726,7 +733,7 @@ export class BulkWriter { */ update( documentRef: firestore.DocumentReference, - dataOrField: firestore.UpdateData | string | FieldPath, + dataOrField: firestore.UpdateData | string | FieldPath, ...preconditionOrValues: Array< {lastUpdateTime?: Timestamp} | unknown | string | FieldPath > diff --git a/dev/src/document-change.ts b/dev/src/document-change.ts index 66b2cbc56..25dba94ca 100644 --- a/dev/src/document-change.ts +++ b/dev/src/document-change.ts @@ -27,7 +27,7 @@ export type DocumentChangeType = 'added' | 'removed' | 'modified'; * @class DocumentChange */ export class DocumentChange - implements firestore.DocumentChange + implements firestore.DocumentChange { private readonly _type: DocumentChangeType; private readonly _document: QueryDocumentSnapshot; diff --git a/dev/src/reference.ts b/dev/src/reference.ts index a54eeedb1..575f64b21 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -132,6 +132,7 @@ export class DocumentReference * * @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, @@ -372,7 +373,7 @@ export class DocumentReference * }); * ``` */ - create(data: T): Promise { + create(data: firestore.WithFieldValue): Promise { const writeBatch = new WriteBatch(this._firestore); return writeBatch .create(this, data) @@ -413,8 +414,11 @@ export class DocumentReference .then(([writeResult]) => writeResult); } - set(data: Partial, options: firestore.SetOptions): Promise; - set(data: T): Promise; + set( + data: firestore.PartialWithFieldValue, + options: firestore.SetOptions + ): Promise; + set(data: firestore.WithFieldValue): Promise; /** * Writes to the document referred to by this DocumentReference. If the * document does not yet exist, it will be created. If you pass @@ -441,14 +445,16 @@ export class DocumentReference * ``` */ set( - data: T | Partial, + data: firestore.PartialWithFieldValue, options?: firestore.SetOptions ): Promise { - 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); + } + return writeBatch.commit().then(([writeResult]) => writeResult); } /** @@ -483,7 +489,7 @@ export class DocumentReference * ``` */ update( - dataOrField: firestore.UpdateData | string | firestore.FieldPath, + dataOrField: firestore.UpdateData | string | firestore.FieldPath, ...preconditionOrValues: Array< unknown | string | firestore.FieldPath | firestore.Precondition > @@ -2691,7 +2697,7 @@ export class CollectionReference * }); * ``` */ - add(data: T): Promise> { + add(data: firestore.WithFieldValue): Promise> { const firestoreData = this._queryOptions.converter.toFirestore(data); validateDocumentData( 'data', diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index b1bba4bc4..bc5fb86c3 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -218,17 +218,23 @@ export class Transaction implements firestore.Transaction { * }); * ``` */ - create(documentRef: firestore.DocumentReference, data: T): Transaction { + create( + documentRef: firestore.DocumentReference, + data: firestore.WithFieldValue + ): Transaction { this._writeBatch.create(documentRef, data); return this; } set( documentRef: firestore.DocumentReference, - data: Partial, + data: firestore.PartialWithFieldValue, options: firestore.SetOptions ): Transaction; - set(documentRef: firestore.DocumentReference, data: T): Transaction; + set( + documentRef: firestore.DocumentReference, + data: firestore.WithFieldValue + ): Transaction; /** * Writes to the document referred to by the provided * [DocumentReference]{@link DocumentReference}. If the document @@ -260,10 +266,14 @@ export class Transaction implements firestore.Transaction { */ set( documentRef: firestore.DocumentReference, - data: T | Partial, + data: firestore.PartialWithFieldValue, 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); + } return this; } @@ -309,7 +319,7 @@ export class Transaction implements firestore.Transaction { */ update( documentRef: firestore.DocumentReference, - dataOrField: firestore.UpdateData | string | firestore.FieldPath, + dataOrField: firestore.UpdateData | string | firestore.FieldPath, ...preconditionOrValues: Array< firestore.Precondition | unknown | string | firestore.FieldPath > diff --git a/dev/src/types.ts b/dev/src/types.ts index 64fe05b38..33bf17e9b 100644 --- a/dev/src/types.ts +++ b/dev/src/types.ts @@ -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'; @@ -114,7 +118,7 @@ export type RBTree = any; * @internal */ const defaultConverterObj: FirestoreDataConverter = { - toFirestore(modelObject: DocumentData): DocumentData { + toFirestore(modelObject: WithFieldValue): DocumentData { return modelObject; }, fromFirestore(snapshot: QueryDocumentSnapshot): DocumentData { diff --git a/dev/src/util.ts b/dev/src/util.ts index 20045c153..0b15be22c 100644 --- a/dev/src/util.ts +++ b/dev/src/util.ts @@ -28,14 +28,14 @@ import * as gapicConfig from './v1/firestore_client_config.json'; */ export class Deferred { promise: Promise; - resolve: (value?: R | Promise) => void = () => {}; - reject: (reason?: Error) => void = () => {}; + resolve: (value: R | Promise) => void = () => {}; + reject: (reason: Error) => void = () => {}; constructor() { - this.promise = new Promise( + this.promise = new Promise( ( - resolve: (value?: R | Promise) => void, - reject: (reason?: Error) => void + resolve: (value: R | Promise) => void, + reject: (reason: Error) => void ) => { this.resolve = resolve; this.reject = reject; diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts index 7f48940a6..287087de9 100644 --- a/dev/src/write-batch.ts +++ b/dev/src/write-batch.ts @@ -189,9 +189,14 @@ export class WriteBatch implements firestore.WriteBatch { * }); * ``` */ - create(documentRef: firestore.DocumentReference, data: T): WriteBatch { + create( + documentRef: firestore.DocumentReference, + data: firestore.WithFieldValue + ): WriteBatch { const ref = validateDocumentReference('documentRef', documentRef); - const firestoreData = ref._converter.toFirestore(data); + const firestoreData = ref._converter.toFirestore( + data as firestore.WithFieldValue + ); validateDocumentData( 'data', firestoreData, @@ -274,14 +279,12 @@ export class WriteBatch implements firestore.WriteBatch { set( documentRef: firestore.DocumentReference, - data: Partial, + data: firestore.PartialWithFieldValue, options: firestore.SetOptions ): WriteBatch; - set(documentRef: firestore.DocumentReference, data: T): WriteBatch; set( documentRef: firestore.DocumentReference, - data: T | Partial, - options?: firestore.SetOptions + data: firestore.WithFieldValue ): WriteBatch; /** * Write to the document referred to by the provided @@ -316,12 +319,12 @@ export class WriteBatch implements firestore.WriteBatch { */ set( documentRef: firestore.DocumentReference, - data: T | Partial, + data: firestore.PartialWithFieldValue, 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) { @@ -415,7 +418,7 @@ export class WriteBatch implements firestore.WriteBatch { */ update( documentRef: firestore.DocumentReference, - dataOrField: firestore.UpdateData | string | firestore.FieldPath, + dataOrField: firestore.UpdateData | string | firestore.FieldPath, ...preconditionOrValues: Array< | {lastUpdateTime?: firestore.Timestamp} | unknown @@ -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).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( @@ -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}; 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]); diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 11e45e1aa..797de2c4f 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -12,7 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {QuerySnapshot, DocumentData} from '@google-cloud/firestore'; +import { + QuerySnapshot, + DocumentData, + WithFieldValue, + PartialWithFieldValue, + SetOptions, +} from '@google-cloud/firestore'; import {describe, it, before, beforeEach, afterEach} from 'mocha'; import {expect, use} from 'chai'; @@ -444,7 +450,7 @@ describe('DocumentReference class', () => { }); it('has set() method', () => { - const allSupportedTypesObject = { + const allSupportedTypesObject: {[field: string]: unknown} = { stringValue: 'a', trueValue: true, falseValue: false, @@ -474,7 +480,7 @@ describe('DocumentReference class', () => { .then(doc => { const data = doc.data()!; expect(data.pathValue.path).to.equal( - allSupportedTypesObject.pathValue.path + (allSupportedTypesObject.pathValue as DocumentReference).path ); delete data.pathValue; delete allSupportedTypesObject.pathValue; @@ -1225,6 +1231,41 @@ describe('DocumentReference class', () => { expect(post).to.not.be.undefined; expect(post!.toString()).to.equal('post, by author'); }); + + it('supports primitive types with valid converter', async () => { + type Primitive = number; + const primitiveConverter = { + toFirestore(value: Primitive): DocumentData { + return {value}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): Primitive { + const data = snapshot.data(); + return data.value; + }, + }; + + type ArrayValue = number[]; + const arrayConverter = { + toFirestore(value: ArrayValue): DocumentData { + return {values: value}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): ArrayValue { + const data = snapshot.data(); + return data.values; + }, + }; + + const coll = firestore.collection('tests'); + const ref = coll.doc('number').withConverter(primitiveConverter); + await ref.set(3); + const result = await ref.get(); + expect(result.data()).to.equal(3); + + const ref2 = coll.doc('array').withConverter(arrayConverter); + await ref2.set([1, 2, 3]); + const result2 = await ref2.get(); + expect(result2.data()).to.deep.equal([1, 2, 3]); + }); }); describe('Query class', () => { @@ -2908,7 +2949,11 @@ describe('Bundle building', () => { ref1.set({name: '1', sort: 1, value: 'string value'}), ref2.set({name: '2', sort: 2, value: 42}), ref3.set({name: '3', sort: 3, value: {nested: 'nested value'}}), - ref4.set({name: '4', sort: 4, value: FieldValue.serverTimestamp()}), + ref4.set({ + name: '4', + sort: 4, + value: FieldValue.serverTimestamp(), + }), ]); }); @@ -3045,3 +3090,807 @@ describe('Bundle building', () => { expect(bundledDoc).to.deep.equal(expected); }); }); + +describe('Types test', () => { + let firestore: Firestore; + let randomCol: CollectionReference; + let doc: DocumentReference; + + class TestObject { + constructor( + readonly outerString: string, + readonly outerArr: string[], + readonly nested: { + innerNested: { + innerNestedNum: number; + }; + innerArr: number[]; + timestamp: Timestamp; + } + ) {} + } + + const testConverter = { + toFirestore(testObj: WithFieldValue) { + return {...testObj}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): TestObject { + const data = snapshot.data(); + return new TestObject(data.outerString, data.outerArr, data.nested); + }, + }; + + const initialData = { + outerString: 'foo', + outerArr: [], + nested: { + innerNested: { + innerNestedNum: 2, + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }; + + beforeEach(async () => { + firestore = new Firestore({}); + randomCol = getTestRoot(firestore); + doc = randomCol.doc(); + + await doc.set(initialData); + }); + + afterEach(() => verifyInstance(firestore)); + + describe('Nested partial support', () => { + const testConverterMerge = { + toFirestore( + testObj: PartialWithFieldValue, + options?: SetOptions + ) { + if (options) { + expect(testObj).to.not.be.an.instanceOf(TestObject); + } else { + expect(testObj).to.be.an.instanceOf(TestObject); + } + return {...testObj}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): TestObject { + const data = snapshot.data(); + return new TestObject(data.outerString, data.outerArr, data.nested); + }, + }; + + it('supports FieldValues', async () => { + const ref = doc.withConverter(testConverterMerge); + + // Allow Field Values in nested partials. + await ref.set( + { + outerString: FieldValue.delete(), + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }, + {merge: true} + ); + + // Allow setting FieldValue on entire object field. + await ref.set( + { + nested: FieldValue.delete(), + }, + {merge: true} + ); + }); + + it('validates types in outer and inner fields', async () => { + const ref = doc.withConverter(testConverterMerge); + + // Check top-level fields. + await ref.set( + { + // @ts-expect-error Should fail to transpile. + outerString: 3, + // @ts-expect-error Should fail to transpile. + outerArr: null, + }, + {merge: true} + ); + + // Check nested fields. + await ref.set( + { + nested: { + innerNested: { + // @ts-expect-error Should fail to transpile. + innerNestedNum: 'string', + }, + // @ts-expect-error Should fail to transpile. + innerArr: null, + }, + }, + {merge: true} + ); + await ref.set( + { + // @ts-expect-error Should fail to transpile. + nested: 3, + }, + {merge: true} + ); + }); + + it('checks for nonexistent properties', async () => { + const ref = doc.withConverter(testConverterMerge); + // Top-level property. + await ref.set( + { + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }, + {merge: true} + ); + + // Nested property + await ref.set( + { + nested: { + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }, + }, + {merge: true} + ); + }); + + it('allows omitting fields', async () => { + const ref = doc.withConverter(testConverterMerge); + // Omit outer fields. + await ref.set( + { + outerString: '', + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }, + {merge: true} + ); + + // Omit inner fields + await ref.set( + { + outerString: '', + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + timestamp: FieldValue.serverTimestamp(), + }, + }, + {merge: true} + ); + }); + }); + + describe('NestedPartial', () => { + const testConverterMerge = { + toFirestore( + testObj: PartialWithFieldValue, + options?: SetOptions + ) { + if (options) { + expect(testObj).to.not.be.an.instanceOf(TestObject); + } else { + expect(testObj).to.be.an.instanceOf(TestObject); + } + return {...testObj}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): TestObject { + const data = snapshot.data(); + return new TestObject(data.outerString, data.outerArr, data.nested); + }, + }; + + it('supports FieldValues', async () => { + const ref = doc.withConverter(testConverterMerge); + + // Allow Field Values in nested partials. + await ref.set( + { + outerString: FieldValue.delete(), + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }, + {merge: true} + ); + + // Allow setting FieldValue on entire object field. + await ref.set( + { + nested: FieldValue.delete(), + }, + {merge: true} + ); + }); + + it('validates types in outer and inner fields', async () => { + const ref = doc.withConverter(testConverterMerge); + + // Check top-level fields. + await ref.set( + { + // @ts-expect-error Should fail to transpile. + outerString: 3, + // @ts-expect-error Should fail to transpile. + outerArr: null, + }, + {merge: true} + ); + + // Check nested fields. + await ref.set( + { + nested: { + innerNested: { + // @ts-expect-error Should fail to transpile. + innerNestedNum: 'string', + }, + // @ts-expect-error Should fail to transpile. + innerArr: null, + }, + }, + {merge: true} + ); + await ref.set( + { + // @ts-expect-error Should fail to transpile. + nested: 3, + }, + {merge: true} + ); + }); + + it('checks for nonexistent properties', async () => { + const ref = doc.withConverter(testConverterMerge); + // Top-level property. + await ref.set( + { + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }, + {merge: true} + ); + + // Nested property + await ref.set( + { + nested: { + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }, + }, + {merge: true} + ); + }); + }); + + describe('WithFieldValue', () => { + it('supports FieldValues', async () => { + const ref = doc.withConverter(testConverter); + + // Allow Field Values and nested partials. + await ref.set({ + outerString: 'foo', + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('requires all outer fields to be present', async () => { + const ref = doc.withConverter(testConverter); + + // @ts-expect-error Should fail to transpile. + await ref.set({ + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('requires all inner fields to be present', async () => { + const ref = doc.withConverter(testConverter); + + await ref.set({ + outerString: '', + outerArr: [], + // @ts-expect-error Should fail to transpile. + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('validates inner and outer fields', async () => { + const ref = doc.withConverter(testConverter); + + await ref.set({ + outerString: 'foo', + // @ts-expect-error Should fail to transpile. + outerArr: 2, + nested: { + innerNested: { + // @ts-expect-error Should fail to transpile. + innerNestedNum: 'string', + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('checks for nonexistent properties', async () => { + const ref = doc.withConverter(testConverter); + + // Top-level nonexistent fields should error + await ref.set({ + outerString: 'foo', + // @ts-expect-error Should fail to transpile. + outerNum: 3, + outerArr: [], + nested: { + innerNested: { + innerNestedNum: 2, + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + + // Nested nonexistent fields should error + await ref.set({ + outerString: 'foo', + outerNum: 3, + outerArr: [], + nested: { + innerNested: { + // @ts-expect-error Should fail to transpile. + nonexistent: 'string', + innerNestedNum: 2, + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('allows certain types for not others', async () => { + const withTryCatch = async ( + fn: () => Promise + ): Promise => { + try { + await fn(); + } catch { + // This is expected. + } + }; + + // These tests exist to establish which object types are allowed to be + // passed in by default when `T = DocumentData`. Some objects extend + // the Javascript `{}`, which is why they're allowed whereas others + // throw an error. + // @ts-expect-error This should fail to transpile. + await withTryCatch(() => doc.set(1)); + // @ts-expect-error This should fail to transpile. + await withTryCatch(() => doc.set('foo')); + // @ts-expect-error This should fail to transpile. + await withTryCatch(() => doc.set(false)); + // @ts-expect-error This should fail to transpile. + await withTryCatch(() => doc.set(undefined)); + // @ts-expect-error This should fail to transpile. + await withTryCatch(() => doc.set(null)); + await withTryCatch(() => doc.set([0])); + await withTryCatch(() => doc.set(new Set())); + await withTryCatch(() => doc.set(new Map())); + }); + + describe('used as a type', () => { + class ObjectWrapper { + withFieldValueT(value: WithFieldValue): WithFieldValue { + return value; + } + + withPartialFieldValueT( + value: PartialWithFieldValue + ): PartialWithFieldValue { + return value; + } + + // Wrapper to avoid having Firebase types in non-Firebase code. + withT(value: T): void { + this.withFieldValueT(value); + } + + // Wrapper to avoid having Firebase types in non-Firebase code. + withPartialT(value: Partial): void { + this.withPartialFieldValueT(value); + } + } + + it('supports passing in the object as `T`', () => { + interface Foo { + id: string; + foo: number; + } + const foo = new ObjectWrapper(); + foo.withFieldValueT({id: '', foo: FieldValue.increment(1)}); + foo.withPartialFieldValueT({foo: FieldValue.increment(1)}); + foo.withT({id: '', foo: 1}); + foo.withPartialT({foo: 1}); + }); + + it('does not allow primitive types to use FieldValue', () => { + type Bar = number; + const bar = new ObjectWrapper(); + // @ts-expect-error This should fail to transpile. + bar.withFieldValueT(FieldValue.increment(1)); + // @ts-expect-error This should fail to transpile. + bar.withPartialFieldValueT(FieldValue.increment(1)); + }); + }); + }); + + describe('UpdateData', () => { + it('supports FieldValues', async () => { + const ref = doc.withConverter(testConverter); + await ref.update({ + outerString: FieldValue.delete(), + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(2), + }, + innerArr: FieldValue.arrayUnion(3), + }, + }); + }); + + it('validates inner and outer fields', async () => { + const ref = doc.withConverter(testConverter); + await ref.update({ + // @ts-expect-error Should fail to transpile. + outerString: 3, + nested: { + innerNested: { + // @ts-expect-error Should fail to transpile. + innerNestedNum: 'string', + }, + // @ts-expect-error Should fail to transpile. + innerArr: 2, + }, + }); + }); + + it('supports string-separated fields', async () => { + const ref = doc.withConverter(testConverter); + await ref.update({ + // @ts-expect-error Should fail to transpile. + outerString: 3, + // @ts-expect-error Should fail to transpile. + 'nested.innerNested.innerNestedNum': 'string', + // @ts-expect-error Should fail to transpile. + 'nested.innerArr': 3, + 'nested.timestamp': FieldValue.serverTimestamp(), + }); + + // String comprehension works in nested fields. + await ref.update({ + nested: { + innerNested: { + // @ts-expect-error Should fail to transpile. + innerNestedNum: 'string', + }, + // @ts-expect-error Should fail to transpile. + innerArr: 3, + }, + }); + }); + + it('supports optional fields', async () => { + interface TestObjectOptional { + optionalStr?: string; + nested?: { + requiredStr: string; + }; + } + + const testConverterOptional = { + toFirestore(testObj: WithFieldValue) { + return {...testObj}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): TestObjectOptional { + const data = snapshot.data(); + return { + optionalStr: data.optionalStr, + nested: data.nested, + }; + }, + }; + + const ref = doc.withConverter(testConverterOptional); + + await ref.update({ + optionalStr: 'foo', + }); + await ref.update({ + optionalStr: 'foo', + }); + + await ref.update({ + nested: { + requiredStr: 'foo', + }, + }); + await ref.update({ + 'nested.requiredStr': 'foo', + }); + }); + + it('supports null fields', async () => { + interface TestObjectOptional { + optionalStr?: string; + nested?: { + strOrNull: string | null; + }; + } + + const testConverterOptional = { + toFirestore(testObj: WithFieldValue) { + return {...testObj}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): TestObjectOptional { + const data = snapshot.data(); + return { + optionalStr: data.optionalStr, + nested: data.nested, + }; + }, + }; + const ref = doc.withConverter(testConverterOptional); + + await ref.update({ + nested: { + strOrNull: null, + }, + }); + await ref.update({ + 'nested.strOrNull': null, + }); + }); + + it('supports union fields', async () => { + interface TestObjectUnion { + optionalStr?: string; + nested?: + | { + requiredStr: string; + } + | {requiredNumber: number}; + } + + const testConverterUnion = { + toFirestore(testObj: WithFieldValue) { + return {...testObj}; + }, + fromFirestore(snapshot: QueryDocumentSnapshot): TestObjectUnion { + const data = snapshot.data(); + return { + optionalStr: data.optionalStr, + nested: data.nested, + }; + }, + }; + + const ref = doc.withConverter(testConverterUnion); + + await ref.update({ + nested: { + requiredStr: 'foo', + }, + }); + + await ref.update({ + 'nested.requiredStr': 'foo', + }); + await ref.update({ + // @ts-expect-error Should fail to transpile. + 'nested.requiredStr': 1, + }); + + await ref.update({ + 'nested.requiredNumber': 1, + }); + + await ref.update({ + // @ts-expect-error Should fail to transpile. + 'nested.requiredNumber': 'foo', + }); + await ref.update({ + // @ts-expect-error Should fail to transpile. + 'nested.requiredNumber': null, + }); + }); + + it('checks for nonexistent fields', async () => { + const ref = doc.withConverter(testConverter); + + // Top-level fields. + await ref.update({ + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }); + + // Nested Fields. + await ref.update({ + nested: { + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }, + }); + + // String fields. + await ref.update({ + // @ts-expect-error Should fail to transpile. + nonexistent: 'foo', + }); + await ref.update({ + // @ts-expect-error Should fail to transpile. + 'nested.nonexistent': 'foo', + }); + }); + }); + + describe('methods', () => { + it('CollectionReference.add()', async () => { + const ref = randomCol.withConverter(testConverter); + + // Requires all fields to be present + // @ts-expect-error Should fail to transpile. + await ref.add({ + outerArr: [], + nested: { + innerNested: { + innerNestedNum: 2, + }, + innerArr: [], + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('WriteBatch.set()', () => { + const ref = doc.withConverter(testConverter); + const batch = firestore.batch(); + + // Requires full object if {merge: true} is not set. + // @ts-expect-error Should fail to transpile. + batch.set(ref, { + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + + batch.set( + ref, + { + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }, + {merge: true} + ); + }); + + it('WriteBatch.update()', () => { + const ref = doc.withConverter(testConverter); + const batch = firestore.batch(); + + batch.update(ref, { + outerArr: [], + nested: { + 'innerNested.innerNestedNum': FieldValue.increment(1), + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + + it('Transaction.set()', async () => { + const ref = doc.withConverter(testConverter); + + return firestore.runTransaction(async tx => { + // Requires full object if {merge: true} is not set. + // @ts-expect-error Should fail to transpile. + tx.set(ref, { + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + + tx.set( + ref, + { + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }, + {merge: true} + ); + }); + }); + + it('Transaction.update()', async () => { + const ref = doc.withConverter(testConverter); + + return firestore.runTransaction(async tx => { + tx.update(ref, { + outerArr: [], + nested: { + innerNested: { + innerNestedNum: FieldValue.increment(1), + }, + innerArr: FieldValue.arrayUnion(2), + timestamp: FieldValue.serverTimestamp(), + }, + }); + }); + }); + }); +}); diff --git a/dev/test/document.ts b/dev/test/document.ts index 042d0d418..43d83fd4a 100644 --- a/dev/test/document.ts +++ b/dev/test/document.ts @@ -1308,56 +1308,28 @@ describe('set document', () => { }); it('validates merge option', () => { - expect(() => { - firestore - .doc('collectionId/documentId') - .set({foo: 'bar'}, 'foo' as InvalidApiUsage); - }).to.throw( - 'Value for argument "options" is not a valid set() options argument. Input is not an object.' - ); - - expect(() => { - firestore.doc('collectionId/documentId').set( - {foo: 'bar'}, - { - merge: 42 as InvalidApiUsage, - } - ); - }).to.throw( - 'Value for argument "options" is not a valid set() options argument. "merge" is not a boolean.' - ); - expect(() => { firestore.doc('collectionId/documentId').set( {foo: 'bar'}, { - mergeFields: 42 as InvalidApiUsage, + mergeFields: ['foobar'], } ); - }).to.throw( - 'Value for argument "options" is not a valid set() options argument. "mergeFields" is not an array.' - ); + }).to.throw('Input data is missing for field "foobar".'); expect(() => { firestore.doc('collectionId/documentId').set( {foo: 'bar'}, { - mergeFields: [null as InvalidApiUsage], + mergeFields: ['foobar..'], } ); }).to.throw( - 'Value for argument "options" is not a valid set() options argument. "mergeFields" is not valid: Element at index 0 is not a valid field path. Paths can only be specified as strings or via a FieldPath object.' + 'Value for argument "options" is not a valid set() options argument. ' + + '"mergeFields" is not valid: Element at index 0 is not a valid ' + + 'field path. Paths must not contain ".." in them.' ); - expect(() => { - firestore.doc('collectionId/documentId').set( - {foo: 'bar'}, - { - mergeFields: ['foobar'], - } - ); - }).to.throw('Input data is missing for field "foobar".'); - expect(() => { firestore .doc('collectionId/documentId') diff --git a/dev/test/index.ts b/dev/test/index.ts index 68bf967cd..a3cf866fe 100644 --- a/dev/test/index.ts +++ b/dev/test/index.ts @@ -262,7 +262,7 @@ const allSupportedTypesInput = { bytesValue: Buffer.from([0x1, 0x2]), }; -const allSupportedTypesOutput = { +const allSupportedTypesOutput: {[field: string]: unknown} = { stringValue: 'a', trueValue: true, falseValue: false, @@ -676,7 +676,7 @@ describe('snapshot_() method', () => { // Deep Equal doesn't support matching instances of DocumentRefs, so we // compare them manually and remove them from the resulting object. expect(actualObject.get('pathValue').formattedName).to.equal( - expected.pathValue.formattedName + (expected.pathValue as Firestore.DocumentReference).formattedName ); const data = actualObject.data()!; delete data.pathValue; diff --git a/dev/test/pool.ts b/dev/test/pool.ts index a78a31631..8a377de68 100644 --- a/dev/test/pool.ts +++ b/dev/test/pool.ts @@ -222,7 +222,7 @@ describe('Client pool', () => { }); it('garbage collection calls destructor', () => { - const garbageCollect = new Deferred(); + const garbageCollect = new Deferred(); const clientPool = new ClientPool<{}>( 1, diff --git a/dev/test/util/helpers.ts b/dev/test/util/helpers.ts index 04922661e..30740d59c 100644 --- a/dev/test/util/helpers.ts +++ b/dev/test/util/helpers.ts @@ -12,7 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {DocumentData, Settings, SetOptions} from '@google-cloud/firestore'; +import { + DocumentData, + Settings, + SetOptions, + PartialWithFieldValue, +} from '@google-cloud/firestore'; import {expect} from 'chai'; import * as extend from 'extend'; @@ -345,8 +350,11 @@ export const postConverter = { }; export const postConverterMerge = { - toFirestore(post: Partial, options?: SetOptions): DocumentData { - if (options && (options.merge || options.mergeFields)) { + toFirestore( + post: PartialWithFieldValue, + options?: SetOptions + ): DocumentData { + if (options) { expect(post).to.not.be.an.instanceOf(Post); } else { expect(post).to.be.an.instanceof(Post); diff --git a/package.json b/package.json index 0e4ad2b70..0bef4e09e 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "proxyquire": "^2.1.3", "sinon": "^12.0.0", "ts-node": "^10.0.0", - "typescript": "3.8.3", + "typescript": "^4.4.3", "through2": "^4.0.0" } } diff --git a/tsconfig.json b/tsconfig.json index e70989a99..fbe20fbd0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,7 +6,8 @@ "lib": [ "es2016", "dom" - ] + ], + "useUnknownInCatchVariables": false, }, "include": [ "dev/src/*.d.ts", diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 558ae812a..c2090f38a 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -27,10 +27,35 @@ declare namespace FirebaseFirestore { */ export type DocumentData = {[field: string]: any}; + /** + * Similar to Typescript's `Partial`, but allows nested fields to be + * omitted and FieldValues to be passed in as property values. + */ + export type PartialWithFieldValue = + | Partial + | (T extends Primitive + ? T + : T extends {} + ? {[K in keyof T]?: PartialWithFieldValue | FieldValue} + : never); + + /** + * Allows FieldValues to be passed in as a property value while maintaining + * type safety. + */ + export type WithFieldValue = + | T + | (T extends Primitive + ? T + : T extends {} + ? {[K in keyof T]: WithFieldValue | FieldValue} + : never); + /** * Update data (for use with [update]{@link DocumentReference#update}) * that contains paths mapped to values. Fields that contain dots reference - * nested fields within the document. + * nested fields within the document. FieldValues can be passed in + * as property values. * * You can update a top-level field in your document by using the field name * as a key (e.g. `foo`). The provided value completely replaces the contents @@ -40,7 +65,72 @@ declare namespace FirebaseFirestore { * key (e.g. `foo.bar`). This nested field update replaces the contents at * `bar` but does not modify other data under `foo`. */ - export type UpdateData = {[fieldPath: string]: any}; + export type UpdateData = T extends Primitive + ? T + : T extends {} + ? {[K in keyof T]?: UpdateData | FieldValue} & NestedUpdateFields + : Partial; + + /** Primitive types. */ + export type Primitive = string | number | boolean | undefined | null; + + /** + * For each field (e.g. 'bar'), find all nested keys (e.g. {'bar.baz': T1, + * 'bar.qux': T2}). Intersect them together to make a single map containing + * all possible keys that are all marked as optional + */ + export type NestedUpdateFields> = + UnionToIntersection< + { + [K in keyof T & string]: ChildUpdateFields; + }[keyof T & string] // Also include the generated prefix-string keys. + >; + + /** + * Helper for calculating the nested fields for a given type T1. This is needed + * to distribute union types such as `undefined | {...}` (happens for optional + * props) or `{a: A} | {b: B}`. + * + * In this use case, `V` is used to distribute the union types of `T[K]` on + * `Record`, since `T[K]` is evaluated as an expression and not distributed. + * + * See https://www.typescriptlang.org/docs/handbook/advanced-types.html#distributive-conditional-types + */ + export type ChildUpdateFields = + // Only allow nesting for map values + V extends Record + ? // Recurse into the map and add the prefix in front of each key + // (e.g. Prefix 'bar.' to create: 'bar.baz' and 'bar.qux'. + AddPrefixToKeys> + : // UpdateData is always a map of values. + never; + + /** + * Returns a new map where every key is prefixed with the outer key appended + * to a dot. + */ + export type AddPrefixToKeys< + Prefix extends string, + T extends Record + > = + // Remap K => Prefix.K. See https://www.typescriptlang.org/docs/handbook/2/mapped-types.html#key-remapping-via-as + {[K in keyof T & string as `${Prefix}.${K}`]+?: T[K]}; + + /** + * Given a union type `U = T1 | T2 | ...`, returns an intersected type + * `(T1 & T2 & ...)`. + * + * Uses distributive conditional types and inference from conditional types. + * This works because multiple candidates for the same type variable in + * contra-variant positions causes an intersection type to be inferred. + * https://www.typescriptlang.org/docs/handbook/advanced-types.html#type-inference-in-conditional-types + * https://stackoverflow.com/questions/50374908/transform-union-type-to-intersection-type + */ + export type UnionToIntersection = ( + U extends unknown ? (k: U) => void : never + ) extends (k: infer I) => void + ? I + : never; /** * Sets or disables the log function for all active Firestore instances. @@ -95,9 +185,27 @@ declare namespace FirebaseFirestore { * into a plain Javascript object (suitable for writing directly to the * Firestore database). To use set() with `merge` and `mergeFields`, * toFirestore() must be defined with `Partial`. + * + * The `WithFieldValue` type extends `T` to also allow FieldValues such + * as `FieldValue.delete()` to be used as property values. */ - toFirestore(modelObject: T): DocumentData; - toFirestore(modelObject: Partial, options: SetOptions): DocumentData; + toFirestore(modelObject: WithFieldValue): DocumentData; + + /** + * Called by the Firestore SDK to convert a custom model object of type T + * into a plain Javascript object (suitable for writing directly to the + * Firestore database). To use set() with `merge` and `mergeFields`, + * toFirestore() must be defined with `Partial`. + * + * The `PartialWithFieldValue` type extends `Partial` to allow + * FieldValues such as `FieldValue.delete()` to be used as property values. + * It also supports nested `Partial` by allowing nested fields to be + * omitted. + */ + toFirestore( + modelObject: PartialWithFieldValue, + options: SetOptions + ): DocumentData; /** * Called by the Firestore SDK to convert Firestore data into an object of @@ -492,7 +600,10 @@ declare namespace FirebaseFirestore { * @param data The object data to serialize as the document. * @return This `Transaction` instance. Used for chaining method calls. */ - create(documentRef: DocumentReference, data: T): Transaction; + create( + documentRef: DocumentReference, + data: WithFieldValue + ): Transaction; /** * Writes to the document referred to by the provided `DocumentReference`. @@ -506,10 +617,13 @@ declare namespace FirebaseFirestore { */ set( documentRef: DocumentReference, - data: Partial, + data: PartialWithFieldValue, options: SetOptions ): Transaction; - set(documentRef: DocumentReference, data: T): Transaction; + set( + documentRef: DocumentReference, + data: WithFieldValue + ): Transaction; /** * Updates fields in the document referred to by the provided @@ -525,9 +639,9 @@ declare namespace FirebaseFirestore { * @param precondition A Precondition to enforce on this update. * @return This `Transaction` instance. Used for chaining method calls. */ - update( - documentRef: DocumentReference, - data: UpdateData, + update( + documentRef: DocumentReference, + data: UpdateData, precondition?: Precondition ): Transaction; @@ -590,7 +704,10 @@ declare namespace FirebaseFirestore { * write fails, the promise is rejected with a * [BulkWriterError]{@link BulkWriterError}. */ - create(documentRef: DocumentReference, data: T): Promise; + create( + documentRef: DocumentReference, + data: WithFieldValue + ): Promise; /** * Delete a document from the database. @@ -636,10 +753,13 @@ declare namespace FirebaseFirestore { */ set( documentRef: DocumentReference, - data: Partial, + data: PartialWithFieldValue, options: SetOptions ): Promise; - set(documentRef: DocumentReference, data: T): Promise; + set( + documentRef: DocumentReference, + data: WithFieldValue + ): Promise; /** * Update fields of the document referred to by the provided @@ -664,9 +784,9 @@ declare namespace FirebaseFirestore { * write fails, the promise is rejected with a * [BulkWriterError]{@link BulkWriterError}. */ - update( - documentRef: DocumentReference, - data: UpdateData, + update( + documentRef: DocumentReference, + data: UpdateData, precondition?: Precondition ): Promise; @@ -835,7 +955,10 @@ declare namespace FirebaseFirestore { * @param data The object data to serialize as the document. * @return This `WriteBatch` instance. Used for chaining method calls. */ - create(documentRef: DocumentReference, data: T): WriteBatch; + create( + documentRef: DocumentReference, + data: WithFieldValue + ): WriteBatch; /** * Write to the document referred to by the provided `DocumentReference`. @@ -849,10 +972,13 @@ declare namespace FirebaseFirestore { */ set( documentRef: DocumentReference, - data: Partial, + data: PartialWithFieldValue, options: SetOptions ): WriteBatch; - set(documentRef: DocumentReference, data: T): WriteBatch; + set( + documentRef: DocumentReference, + data: WithFieldValue + ): WriteBatch; /** * Update fields of the document referred to by the provided @@ -868,9 +994,9 @@ declare namespace FirebaseFirestore { * @param precondition A Precondition to enforce on this update. * @return This `WriteBatch` instance. Used for chaining method calls. */ - update( - documentRef: DocumentReference, - data: UpdateData, + update( + documentRef: DocumentReference, + data: UpdateData, precondition?: Precondition ): WriteBatch; @@ -943,25 +1069,22 @@ declare namespace FirebaseFirestore { * `DocumentReference`, `WriteBatch` and `Transaction`. These calls can be * configured to perform granular merges instead of overwriting the target * documents in their entirety. + * + * @param merge Changes the behavior of a set() call to only replace the + * values specified in its data argument. Fields omitted from the set() call + * remain untouched. + * + * @param mergeFields Changes the behavior of set() calls to only replace + * the specified field paths. Any field path that is not specified is ignored + * and remains untouched. */ - export interface SetOptions { - /** - * Changes the behavior of a set() call to only replace the values specified - * in its data argument. Fields omitted from the set() call remain - * untouched. - */ - readonly merge?: boolean; - - /** - * Changes the behavior of set() calls to only replace the specified field - * paths. Any field path that is not specified is ignored and remains - * untouched. - * - * It is an error to pass a SetOptions object to a set() call that is - * missing a value for any of the fields specified here. - */ - readonly mergeFields?: (string | FieldPath)[]; - } + export type SetOptions = + | { + readonly merge?: boolean; + } + | { + readonly mergeFields?: Array; + }; /** * An options object that can be used to configure the behavior of `getAll()` @@ -1053,7 +1176,7 @@ declare namespace FirebaseFirestore { * @param data The object data to serialize as the document. * @return A Promise resolved with the write time of this create. */ - create(data: T): Promise; + create(data: WithFieldValue): Promise; /** * Writes to the document referred to by this `DocumentReference`. If the @@ -1064,8 +1187,11 @@ declare namespace FirebaseFirestore { * @param options An object to configure the set behavior. * @return A Promise resolved with the write time of this set. */ - set(data: Partial, options: SetOptions): Promise; - set(data: T): Promise; + set( + data: PartialWithFieldValue, + options: SetOptions + ): Promise; + set(data: WithFieldValue): Promise; /** * Updates fields in the document referred to by this `DocumentReference`. @@ -1079,7 +1205,10 @@ declare namespace FirebaseFirestore { * @param precondition A Precondition to enforce on this update. * @return A Promise resolved with the write time of this update. */ - update(data: UpdateData, precondition?: Precondition): Promise; + update( + data: UpdateData, + precondition?: Precondition + ): Promise; /** * Updates fields in the document referred to by this `DocumentReference`. @@ -1690,7 +1819,7 @@ declare namespace FirebaseFirestore { * @return A Promise resolved with a `DocumentReference` pointing to the * newly created document after it has been written to the backend. */ - add(data: T): Promise>; + add(data: WithFieldValue): Promise>; /** * Returns true if this `CollectionReference` is equal to the provided one.