Moment.js does not work for date queries

[jakeleventhal]

Environment details

• OS: Linux b8b01391ee38 4.9.125-linuxkit Fix export documentation. #1 SMP Fri Sep 7 08:20:28 UTC 2018 x86_64 GNU/Linux (this is from the node:latest docker image)
• Node.js version: 12.9.0
• @google-cloud/firestore version: 2.2.8

Steps to reproduce

1. Construct a query on a date field field and pass a moment.js object in as the filter value

firestore.collection('Users').where('JoinedDate', '>=', moment().subtract(1, 'day'))

2. You will get an error that looks like this:

Error: Value for argument "value" is not a valid Firestore document. Couldn't serialize object of type "Moment". Firestore doesn't support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).

Moment.js is such a popular library used for dates that i think an exception should be made for this.

[schmidt-sebastian]

@jakeleventhal Sorry to hear about your trouble.

For now, we only special-case JavaScript's Date (as well as our own Timestamp object). You can convert a moment() date to a JS Date via moment().toDate(). We might consider native support in the future, but would likely need more user feedback/demand before doing so.

[jakeleventhal]

Yeah, we currently do that. It just seems like an unnecessary step. Wouldn't the fix be as simple as adding the following to the where clause check? I was going to make the PR myself but i didn't want to waste my breath if you guys aren't okay with it.

if (typeof val === 'object' && val.constructor.name === 'Moment') {
   val = val.toDate();
}

I understand not wanting to add special case checks for esoteric object types - it's just that moment is used in so many projects.

[schmidt-sebastian]

I talked to some of the folks internally, and we are a little hesitant about adding first party support, especially given existing proposals to add better time management to the Node standard library (such as https://github.com/tc39/proposal-temporal). We can still keep this open as a feature request, which will allow us to gather more feedback.

[bcoe]

perhaps a good compromise would be to add some samples (time permitting) that demonstrate using firestore in conjunction with moment? I personally love moment, but agree that it's probably a bit too opinionated of a dependency to pull in directly.

[schmidt-sebastian]

Just to clarify - we could accept it as an input type even without a dependency. I don't think we would ever return it from a DocumentSnapshot though (which might be a reason to also not accept it as input).

[jakeleventhal]

Right @schmidt-sebastian, @bcoe the sample code i provided above is all that you would need. Just a check on the string type to see if the object is of type "Moment"