-
Notifications
You must be signed in to change notification settings - Fork 888
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
add support for set() with SetOptions when using FirestoreDataConverter #3254
Changes from 16 commits
f6c62da
3927512
1fd7d66
390d49c
4e8ca3c
fd11cd7
4b12457
767c936
940c86d
89f2c8d
a09f5cf
7dafd4a
bcc09ba
f289a76
695d0a4
3c00f96
57ed84b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'firebase': minor | ||
'@firebase/firestore': minor | ||
'@firebase/firestore-types': minor | ||
--- | ||
|
||
Added support for `set()` with merge options when using `FirestoreDataConverter`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<T>`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, suggest backticks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. thanks for the review! |
||
*/ | ||
toFirestore(modelObject: T): DocumentData; | ||
toFirestore(modelObject: Partial<T>, 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<T>( | ||
documentRef: DocumentReference<T>, | ||
data: T, | ||
options?: SetOptions | ||
data: Partial<T>, | ||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry if I'm misreading this, but is SetOptions now required? Do we need a param for it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are adding an overload that allows to specify a |
||
* @return This `Transaction` instance. Used for chaining method calls. | ||
*/ | ||
set<T>(documentRef: DocumentReference<T>, 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<T>( | ||
documentRef: DocumentReference<T>, | ||
data: T, | ||
options?: SetOptions | ||
data: Partial<T>, | ||
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<T>(documentRef: DocumentReference<T>, 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<void>; | ||
set(data: Partial<T>, options: SetOptions): Promise<void>; | ||
|
||
/** | ||
* 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<void>; | ||
|
||
/** | ||
* Updates fields in the document referred to by this `DocumentReference`. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the literal
set()
should have backticks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.