FR: Generic Document Types

[davewasmer]

Describe your environment

• Operating System version: N/A
• Browser version: N/A
• Firebase SDK version: 6.2.0
• Firebase Product: Firestore

Describe the problem

Steps to reproduce:

Currently:

let post = await firebase.firestore().collection('posts').doc('123').get();
post.data().title; // Ideally, should know this is a string
post.data().someNonExistentProperty; // Ideally, should error in Typescript

It would be extremely helpful if the Firestore APIs accepted generic type arguments to describe the shape of items in a given collection:

interface Post {
  title: string
}
let post = await firebase.firestore().collection<Post>('posts').doc('123').get();
post.data().title; // string
post.data().someNonExistentProperty; // TS error

I know I can cast the result of .data():

post.data() as Post

But that's less than ideal, because:

1. I have to remember to cast each and every time, vs. being able to supply generic type args to the .collection() method could ensure types for all document references from that collection, and
2. Casting the .data() call to a type misses out on type improvements to .set() and other methods.

[mikelehen]

Thanks for the suggestion! I can definitely see the appeal of this, and it has come up a couple times before (e.g. here on our JS server SDK), but we currently don't intend to implement this since it would convey a level of type-safety that did not actually exist. To implement it, we would have to do unsafe casts on our side which means we could return data to you that doesn't actually match the type defined.

What we may be able to do in the future is expose some sort of more sophisticated serialization / "object mapping" mechanism, where you could register your model classes with the SDK in such a way that we would know how to properly serialize them to/from Firestore. E.g. you write .toFirestore() and.fromFirestore() methods for your classes and then the SDK could automatically call them as appropriate. This is something we've talked about supporting but it would need some non-trivial design and implementation work, so I'm not sure when we'll get to it.

Sorry I don't have a better answer, but I hope this helps explain where we're at / what we're thinking. Thanks again for the feedback.

[davewasmer]

@mikebonnell thanks for the quick response! Apologies, I searched this repo for any prior discussion, didn't realize there was the other repo you linked to as well.

Not sure if I should bring this up on the other repo, or perhaps this is just one of those "we've discussed it a hundred times, it's just not happening" things, so feel free to let me know, but ...

It seems like default generic args should get the best of both worlds here:

collection<T extends {} = any>() {
  // ...
}

This would allow the API to keep any for the default type, but allow for users to supply their own casting.

I realize that the casting has to happen somewhere - but pushing it into userland code results in a far greater chance for error I'd argue. Users now need to cast every related data access method with the appropriate types everywhere. It ends up being the exact opposite of what the type system helps with: now I can't rely on the TS compiler to actually check the correctness of my code, because I may have forgotten a cast somewhere, and an any may have snuck into my types without me realizing it.

I think I'd also push back on the notion from your linked issue thread, that this might lead to confusion given the schemaless nature of Firestore. The whole concept of types in Typescript is that they do not provide runtime guarantees - TS devs are quite used to this situation. Not saying that the confusion will never arise, but I think the benefits of providing generic types would outweigh the confusion costs easily.

Again - not trying to rehash a dead issue, but if there's room for reconsidering, I'd strongly suggest it.

[mikelehen]

@davewasmer Thanks for the added feedback. I'll bring this up with the team again to revisit the feedback we've gotten (this has come up a few times) and consider solution ideas.

[davewasmer]

Thanks @mikelehen!

[thebrianchen]

Added generic document types in #2240