diff --git a/.changeset/quiet-coats-type.md b/.changeset/quiet-coats-type.md new file mode 100644 index 00000000000..4dd43f683b5 --- /dev/null +++ b/.changeset/quiet-coats-type.md @@ -0,0 +1,7 @@ +--- +'firebase': minor +'@firebase/firestore': minor +'@firebase/firestore-types': minor +--- + +Added support for `set()` with merge options when using `FirestoreDataConverter`. diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 93df8ad400f..518f22d130a 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -7859,9 +7859,11 @@ declare namespace firebase.firestore { /** * 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). + * Firestore database). To use `set()` with `merge` and `mergeFields`, + * `toFirestore()` must be defined with `Partial`. */ toFirestore(modelObject: T): DocumentData; + toFirestore(modelObject: Partial, options: SetOptions): DocumentData; /** * Called by the Firestore SDK to convert Firestore data into an object of @@ -8310,10 +8312,21 @@ declare namespace firebase.firestore { */ set( documentRef: DocumentReference, - data: T, - options?: SetOptions + data: Partial, + options: SetOptions ): Transaction; + /** + * Writes to the document referred to by the provided `DocumentReference`. + * If the document does not exist yet, it will be created. If you pass + * `SetOptions`, the provided data can be merged into the existing document. + * + * @param documentRef A reference to the document to be set. + * @param data An object of the fields and values for the document. + * @return This `Transaction` instance. Used for chaining method calls. + */ + set(documentRef: DocumentReference, data: T): Transaction; + /** * Updates fields in the document referred to by the provided * `DocumentReference`. The update will fail if applied to a document that @@ -8384,10 +8397,21 @@ declare namespace firebase.firestore { */ set( documentRef: DocumentReference, - data: T, - options?: SetOptions + data: Partial, + options: SetOptions ): WriteBatch; + /** + * Writes to the document referred to by the provided `DocumentReference`. + * If the document does not exist yet, it will be created. If you pass + * `SetOptions`, the provided data can be merged into the existing document. + * + * @param documentRef A reference to the document to be set. + * @param data An object of the fields and values for the document. + * @return This `WriteBatch` instance. Used for chaining method calls. + */ + set(documentRef: DocumentReference, data: T): WriteBatch; + /** * Updates fields in the document referred to by the provided * `DocumentReference`. The update will fail if applied to a document that @@ -8566,7 +8590,18 @@ declare namespace firebase.firestore { * @return A Promise resolved once the data has been successfully written * to the backend (Note that it won't resolve while you're offline). */ - set(data: T, options?: SetOptions): Promise; + set(data: Partial, options: SetOptions): Promise; + + /** + * Writes to the document referred to by this `DocumentReference`. If the + * document does not yet exist, it will be created. If you pass + * `SetOptions`, the provided data can be merged into an existing document. + * + * @param data A map of the fields and values for the document. + * @return A Promise resolved once the data has been successfully written + * to the backend (Note that it won't resolve while you're offline). + */ + set(data: T): Promise; /** * Updates fields in the document referred to by this `DocumentReference`. diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index d264cacf795..2346aebf2eb 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -50,6 +50,7 @@ export function setLogLevel(logLevel: LogLevel): void; export interface FirestoreDataConverter { toFirestore(modelObject: T): DocumentData; + toFirestore(modelObject: Partial, options: SetOptions): DocumentData; fromFirestore(snapshot: QueryDocumentSnapshot, options: SnapshotOptions): T; } @@ -146,9 +147,10 @@ export class Transaction { set( documentRef: DocumentReference, - data: T, - options?: SetOptions + data: Partial, + options: SetOptions ): Transaction; + set(documentRef: DocumentReference, data: T): Transaction; update(documentRef: DocumentReference, data: UpdateData): Transaction; update( @@ -166,9 +168,10 @@ export class WriteBatch { set( documentRef: DocumentReference, - data: T, - options?: SetOptions + data: Partial, + options: SetOptions ): WriteBatch; + set(documentRef: DocumentReference, data: T): WriteBatch; update(documentRef: DocumentReference, data: UpdateData): WriteBatch; update( @@ -208,7 +211,8 @@ export class DocumentReference { isEqual(other: DocumentReference): boolean; - set(data: T, options?: SetOptions): Promise; + set(data: Partial, options: SetOptions): Promise; + set(data: T): Promise; update(data: UpdateData): Promise; update( diff --git a/packages/firestore/lite/src/api/reference.ts b/packages/firestore/lite/src/api/reference.ts index dc6736ee66c..fc48f56fb2b 100644 --- a/packages/firestore/lite/src/api/reference.ts +++ b/packages/firestore/lite/src/api/reference.ts @@ -456,16 +456,13 @@ export function setDoc( ): Promise { const ref = cast(reference, DocumentReference); - const [convertedValue] = applyFirestoreDataConverter( - ref._converter, - data, - 'setDoc' - ); + const convertedValue = applyFirestoreDataConverter(ref._converter, data); const dataReader = newUserDataReader(ref.firestore); const parsed = dataReader.parseSetData( 'setDoc', ref._key, convertedValue, + ref._converter !== null, options ); @@ -548,14 +545,16 @@ export function addDoc( const collRef = cast(reference, CollectionReference); const docRef = doc(collRef); - const [convertedValue] = applyFirestoreDataConverter( - collRef._converter, - data, - 'addDoc' - ); + const convertedValue = applyFirestoreDataConverter(collRef._converter, data); const dataReader = newUserDataReader(collRef.firestore); - const parsed = dataReader.parseSetData('addDoc', docRef._key, convertedValue); + const parsed = dataReader.parseSetData( + 'addDoc', + docRef._key, + convertedValue, + docRef._converter !== null, + {} + ); return collRef.firestore ._getDatastore() diff --git a/packages/firestore/lite/src/api/transaction.ts b/packages/firestore/lite/src/api/transaction.ts index 27fa86cf7ea..dcbe0c34eb3 100644 --- a/packages/firestore/lite/src/api/transaction.ts +++ b/packages/firestore/lite/src/api/transaction.ts @@ -95,15 +95,12 @@ export class Transaction implements firestore.Transaction { options?: firestore.SetOptions ): Transaction { const ref = validateReference(documentRef, this._firestore); - const [convertedValue] = applyFirestoreDataConverter( - ref._converter, - value, - 'Transaction.set' - ); + const convertedValue = applyFirestoreDataConverter(ref._converter, value); const parsed = this._dataReader.parseSetData( 'Transaction.set', ref._key, convertedValue, + ref._converter !== null, options ); this._transaction.set(ref._key, parsed); diff --git a/packages/firestore/lite/src/api/write_batch.ts b/packages/firestore/lite/src/api/write_batch.ts index b190ca9d1c2..f012df989ee 100644 --- a/packages/firestore/lite/src/api/write_batch.ts +++ b/packages/firestore/lite/src/api/write_batch.ts @@ -59,16 +59,13 @@ export class WriteBatch implements firestore.WriteBatch { this.verifyNotCommitted(); const ref = validateReference(documentRef, this._firestore); - const [convertedValue] = applyFirestoreDataConverter( - ref._converter, - value, - 'WriteBatch.set' - ); + const convertedValue = applyFirestoreDataConverter(ref._converter, value); const parsed = this._dataReader.parseSetData( 'WriteBatch.set', ref._key, convertedValue, + ref._converter !== null, options ); this._mutations = this._mutations.concat( diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index cdce37dcae3..ecc9656738f 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -726,9 +726,15 @@ export class Transaction implements firestore.Transaction { }); } + set( + documentRef: DocumentReference, + data: Partial, + options: firestore.SetOptions + ): Transaction; + set(documentRef: DocumentReference, data: T): Transaction; set( documentRef: firestore.DocumentReference, - value: T, + value: T | Partial, options?: firestore.SetOptions ): Transaction { validateBetweenNumberOfArgs('Transaction.set', arguments, 2, 3); @@ -738,15 +744,16 @@ export class Transaction implements firestore.Transaction { this._firestore ); options = validateSetOptions('Transaction.set', options); - const [convertedValue, functionName] = applyFirestoreDataConverter( + const convertedValue = applyFirestoreDataConverter( ref._converter, value, - 'Transaction.set' + options ); const parsed = this._firestore._dataReader.parseSetData( - functionName, + 'Transaction.set', ref._key, convertedValue, + ref._converter !== null, options ); this._transaction.set(ref._key, parsed); @@ -825,9 +832,15 @@ export class WriteBatch implements firestore.WriteBatch { constructor(private _firestore: Firestore) {} + set( + documentRef: DocumentReference, + data: Partial, + options: firestore.SetOptions + ): WriteBatch; + set(documentRef: DocumentReference, data: T): WriteBatch; set( documentRef: firestore.DocumentReference, - value: T, + value: T | Partial, options?: firestore.SetOptions ): WriteBatch { validateBetweenNumberOfArgs('WriteBatch.set', arguments, 2, 3); @@ -838,15 +851,16 @@ export class WriteBatch implements firestore.WriteBatch { this._firestore ); options = validateSetOptions('WriteBatch.set', options); - const [convertedValue, functionName] = applyFirestoreDataConverter( + const convertedValue = applyFirestoreDataConverter( ref._converter, value, - 'WriteBatch.set' + options ); const parsed = this._firestore._dataReader.parseSetData( - functionName, + 'WriteBatch.set', ref._key, convertedValue, + ref._converter !== null, options ); this._mutations = this._mutations.concat( @@ -1032,22 +1046,21 @@ export class DocumentReference ); } - set( - value: firestore.DocumentData, - options?: firestore.SetOptions - ): Promise; - set(value: T, options?: firestore.SetOptions): Promise { + set(value: Partial, options: firestore.SetOptions): Promise; + set(value: T): Promise; + set(value: T | Partial, options?: firestore.SetOptions): Promise { validateBetweenNumberOfArgs('DocumentReference.set', arguments, 1, 2); options = validateSetOptions('DocumentReference.set', options); - const [convertedValue, functionName] = applyFirestoreDataConverter( + const convertedValue = applyFirestoreDataConverter( this._converter, value, - 'DocumentReference.set' + options ); const parsed = this.firestore._dataReader.parseSetData( - functionName, + 'DocumentReference.set', this._key, convertedValue, + this._converter !== null, options ); return this._firestoreClient.write( @@ -2584,16 +2597,22 @@ function resultChangeType(type: ChangeType): firestore.DocumentChangeType { export function applyFirestoreDataConverter( converter: UntypedFirestoreDataConverter | null, value: T, - functionName: string -): [firestore.DocumentData, string] { + options?: firestore.SetOptions +): firestore.DocumentData { let convertedValue; if (converter) { - convertedValue = converter.toFirestore(value); - functionName = 'toFirestore() in ' + functionName; + if (options && (options.merge || options.mergeFields)) { + // Cast to `any` in order to satisfy the union type constraint on + // toFirestore(). + // eslint-disable-next-line @typescript-eslint/no-explicit-any + convertedValue = (converter as any).toFirestore(value, options); + } else { + convertedValue = converter.toFirestore(value); + } } else { convertedValue = value as firestore.DocumentData; } - return [convertedValue, functionName]; + return convertedValue; } function contains(obj: object, key: string): obj is { key: unknown } { diff --git a/packages/firestore/src/api/user_data_reader.ts b/packages/firestore/src/api/user_data_reader.ts index bb50802efd8..24d44886315 100644 --- a/packages/firestore/src/api/user_data_reader.ts +++ b/packages/firestore/src/api/user_data_reader.ts @@ -173,6 +173,11 @@ interface ContextSettings { * If not set, elements are treated as if they were outside of arrays. */ readonly arrayElement?: boolean; + /** + * Whether or not a converter was specified in this context. If true, error + * messages will reference the converter when invalid data is provided. + */ + readonly hasConverter?: boolean; } /** A "context" object passed around while parsing user data. */ @@ -258,6 +263,7 @@ export class ParseContext { return createError( reason, this.settings.methodName, + this.settings.hasConverter || false, this.path, this.settings.targetDoc ); @@ -314,6 +320,7 @@ export class UserDataReader { methodName: string, targetDoc: DocumentKey, input: unknown, + hasConverter: boolean, options: firestore.SetOptions = {} ): ParsedSetData { const context = this.createContext( @@ -321,7 +328,8 @@ export class UserDataReader { ? UserDataSource.MergeSet : UserDataSource.Set, methodName, - targetDoc + targetDoc, + hasConverter ); validatePlainObject('Data must be an object, but it was:', context, input); const updateData = parseObject(input, context)!; @@ -494,7 +502,8 @@ export class UserDataReader { private createContext( dataSource: UserDataSource, methodName: string, - targetDoc?: DocumentKey + targetDoc?: DocumentKey, + hasConverter = false ): ParseContext { return new ParseContext( { @@ -502,7 +511,8 @@ export class UserDataReader { methodName, targetDoc, path: FieldPath.emptyPath(), - arrayElement: false + arrayElement: false, + hasConverter }, this.databaseId, this.serializer, @@ -774,7 +784,13 @@ export function fieldPathFromArgument( return fieldPathFromDotSeparatedString(methodName, path); } else { const message = 'Field path arguments must be of type string or FieldPath.'; - throw createError(message, methodName, undefined, targetDoc); + throw createError( + message, + methodName, + /* hasConverter= */ false, + /* path= */ undefined, + targetDoc + ); } } @@ -795,18 +811,30 @@ export function fieldPathFromDotSeparatedString( return fromDotSeparatedString(path)._internalPath; } catch (e) { const message = errorMessage(e); - throw createError(message, methodName, undefined, targetDoc); + throw createError( + message, + methodName, + /* hasConverter= */ false, + /* path= */ undefined, + targetDoc + ); } } function createError( reason: string, methodName: string, + hasConverter: boolean, path?: FieldPath, targetDoc?: DocumentKey ): Error { const hasPath = path && !path.isEmpty(); const hasDocument = targetDoc !== undefined; + let message = `Function ${methodName}() called with invalid data`; + if (hasConverter) { + message += ' (via `toFirestore()`)'; + } + message += '. '; let description = ''; if (hasPath || hasDocument) { @@ -823,7 +851,7 @@ function createError( return new FirestoreError( Code.INVALID_ARGUMENT, - `Function ${methodName}() called with invalid data. ` + reason + description + message + reason + description ); } diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 33685ab2900..097579d9dfd 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -1296,6 +1296,34 @@ apiDescribe('Database', (persistence: boolean) => { } }; + const postConverterMerge = { + toFirestore( + post: Partial, + options?: firestore.SetOptions + ): firestore.DocumentData { + if (options && (options.merge || options.mergeFields)) { + expect(post).to.not.be.an.instanceof(Post); + } else { + expect(post).to.be.an.instanceof(Post); + } + const result: firestore.DocumentData = {}; + if (post.title) { + result.title = post.title; + } + if (post.author) { + result.author = post.author; + } + return result; + }, + fromFirestore( + snapshot: firestore.QueryDocumentSnapshot, + options: firestore.SnapshotOptions + ): Post { + const data = snapshot.data(); + return new Post(data.title, data.author); + } + }; + it('for DocumentReference.withConverter()', () => { return withTestDb(persistence, async db => { const docRef = db @@ -1340,6 +1368,127 @@ apiDescribe('Database', (persistence: boolean) => { }); }); + it('requires the correct converter for Partial usage', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc('some-post') + .withConverter(postConverter); + await ref.set(new Post('walnut', 'author')); + const batch = db.batch(); + expect(() => + batch.set(ref, { title: 'olive' }, { merge: true }) + ).to.throw( + 'Function WriteBatch.set() called with invalid ' + + 'data (via `toFirestore()`). Unsupported field value: undefined ' + + '(found in field author in document posts/some-post)' + ); + }); + }); + + it('WriteBatch.set() supports partials with merge', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc() + .withConverter(postConverterMerge); + await ref.set(new Post('walnut', 'author')); + const batch = db.batch(); + batch.set(ref, { title: 'olive' }, { merge: true }); + await batch.commit(); + const doc = await ref.get(); + expect(doc.get('title')).to.equal('olive'); + expect(doc.get('author')).to.equal('author'); + }); + }); + + it('WriteBatch.set() supports partials with mergeFields', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc() + .withConverter(postConverterMerge); + await ref.set(new Post('walnut', 'author')); + const batch = db.batch(); + batch.set( + ref, + { title: 'olive', author: 'writer' }, + { mergeFields: ['title'] } + ); + await batch.commit(); + const doc = await ref.get(); + expect(doc.get('title')).to.equal('olive'); + expect(doc.get('author')).to.equal('author'); + }); + }); + + it('Transaction.set() supports partials with merge', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc() + .withConverter(postConverterMerge); + await ref.set(new Post('walnut', 'author')); + await db.runTransaction(async tx => { + tx.set(ref, { title: 'olive' }, { merge: true }); + }); + const doc = await ref.get(); + expect(doc.get('title')).to.equal('olive'); + expect(doc.get('author')).to.equal('author'); + }); + }); + + it('Transaction.set() supports partials with mergeFields', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc() + .withConverter(postConverterMerge); + await ref.set(new Post('walnut', 'author')); + await db.runTransaction(async tx => { + tx.set( + ref, + { title: 'olive', author: 'person' }, + { mergeFields: ['title'] } + ); + }); + const doc = await ref.get(); + expect(doc.get('title')).to.equal('olive'); + expect(doc.get('author')).to.equal('author'); + }); + }); + + it('DocumentReference.set() supports partials with merge', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc() + .withConverter(postConverterMerge); + await ref.set(new Post('walnut', 'author')); + await ref.set({ title: 'olive' }, { merge: true }); + const doc = await ref.get(); + expect(doc.get('title')).to.equal('olive'); + expect(doc.get('author')).to.equal('author'); + }); + }); + + it('DocumentReference.set() supports partials with mergeFields', async () => { + return withTestDb(persistence, async db => { + const ref = db + .collection('posts') + .doc() + .withConverter(postConverterMerge); + await ref.set(new Post('walnut', 'author')); + await ref.set( + { title: 'olive', author: 'writer' }, + { mergeFields: ['title'] } + ); + const doc = await ref.get(); + expect(doc.get('title')).to.equal('olive'); + expect(doc.get('author')).to.equal('author'); + }); + }); + it('calls DocumentSnapshot.data() with specified SnapshotOptions', () => { return withTestDb(persistence, async db => { const docRef = db.doc('some/doc').withConverter({