feat: support serialization of Moment.js #879
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #879 +/- ##
==========================================
+ Coverage 94.9% 94.91% +<.01%
==========================================
Files 25 25
Lines 15202 15229 +27
Branches 1128 1134 +6
==========================================
+ Hits 14427 14454 +27
Misses 767 767
Partials 8 8
Continue to review full report at Codecov.
|
| @@ -369,6 +381,8 @@ export function validateUserInput( | |||
| // Ok. | |||
| } else if (value instanceof Timestamp || value instanceof Date) { | |||
| // Ok. | |||
| } else if (isMomentJsType(value)) { | |||
There was a problem hiding this comment.
Just a thought -- is there value in switching the logic here to use switch/case instead of if/else blocks?
There was a problem hiding this comment.
As discussed, this doesn't make sense here since we mix "instanceof" with "typeof" among other things.
|
|
||
| return createInstance(overrides).then(firestore => { | ||
| return firestore.doc('collectionId/documentId').set({ | ||
| moonLanding: new Moment(), |
There was a problem hiding this comment.
All the special effects!
|
Just to be clear: this PR allows us to pass moment objects as field values with set/update operations, but does not allow querying for Timestamp values using moment objects. For the latter we still need to use Is this correct? |
|
@willbattel The serialization logic is the same in all parts of the API. You should be able to use MomentJs types as query constraints. |
Fixes #753
Note that this only adds a dev dependency on the MomentJs types, but not MomentJs itself.