Consider adding `null` to the WardTimestamp definition

[henrymoulton]

Here's something that threw me off a bit, ServerTimestamps can evaluate to null:

If omitted or set to 'none', null will be returned by default until the server value becomes available. https://firebase.google.com/docs/reference/js/firebase.firestore.SnapshotOptions

I found more details here: https://github.com/firebase/firebase-js-sdk/issues/192#issuecomment-336517226

Do you think this should affect the timestamp Type?

edit: I guess not the generated security rule, but perhaps the generated TypeScript type definition?

I've got a list of messages that are declared as:

type Message = {
  text: string,
  createdAt: timestamp,
}

I'm then listening on the messages collection to return the message docs sorted by createdAt

createdAt then is turned into the TS definition: createdAt: Date | WardTimestamp | { isEqual: (other: any) => boolean };

however when using that type inside client-side code, it doesn't include the possibility of it being null

[bijoutrouvaille]

@henrymoulton How interesting. Thanks for posting. This seems like something that should definitely be accounted for in the typings. I'll need to spend a little time researching the problem, and I'll leave the issue open meanwhile.

Have you encountered any run-time errors due to this, by chance? Is there an easy way to reproduce one?

[henrymoulton]

I did because I assumed it was a WardTimestamp object that I could call .toDate() on when in fact it was null. I’ve now added a check.

To repro, if you have a query snapshot running and append a doc which has a Firestore server timestamp appended to it, you should see the documents value initially as null, and then resolved to a value pretty quickly after.

I’m using React Native Firebase on iOS...

[henrymoulton]

Which just to add clarity would be using the iOS SDK to then offer the JS api.

It seems the iOS SDK has the same behaviour: https://firebase.google.com/docs/reference/swift/firebasefirestore/api/reference/Enums/ServerTimestampBehavior

[bijoutrouvaille]

null added to definition. Should be available in 1.4.3. Thanks for reporting.