angularfire icon indicating copy to clipboard operation
angularfire copied to clipboard

AngularFire 7 Document Typing

Open kevin-induro opened this issue 3 years ago • 10 comments

Have we lost document typing with the upgrade to AngularFire 7?

In the past, we were able to do (as shown in the docs) something like afs.doc<Item>('items/1') and that would return DocumentReference<Item> as the type.

Now it looks like the functions that exist are

function doc(firestore: types.FirebaseFirestore, path: string, ...pathSegments: string[]): DocumentReference<DocumentData>;
function doc<T>(reference: types.CollectionReference<T>, path?: string, ...pathSegments: string[]): DocumentReference<T>;
function doc(reference: types.DocumentReference<unknown>, path: string, ...pathSegments: string[]): DocumentReference<DocumentData>;

The only one that is typed is the one that takes in a previous collection reference. Meanwhile, this first is the one we're directed to use in the upgrade documentation and it only uses DocumentData. One of the big benefits to using AngularFire previously over the vanilla Firebase libraries was the fact that DocumentData didn't have to be cast. Has that now been removed?

kevin-induro avatar Sep 07 '21 22:09 kevin-induro

This issue does not seem to follow the issue template. Make sure you provide all the required information.

google-oss-bot avatar Sep 07 '21 22:09 google-oss-bot

Angular: 12.2.4 Firebase: 9.0.1 AngularFire: 7.0.3

kevin-induro avatar Sep 08 '21 13:09 kevin-induro

I should also mention that the above means that the current upgrade documentation is wrong. It states that the alternative to doc in v6 / Compat is

import { doc } from '@angular/fire/firestore';
doc<T>(firestore, 'foo/bar') // DocumentReference<T>

In fact, this gives an error stating Argument of type 'Firestore' is not assignable to parameter of type 'CollectionReference<T>'.

kevin-induro avatar Sep 08 '21 13:09 kevin-induro

It seems Firestore isn't allowing one to override the typings, only allowing types to be inferred if using a converter. Converters are "better" in the sense that you can have real run-time checks to guard you against data that doesn't conform to your expectations but they can be overly verbose.

const fooCollection = collection(firestore, 'foo').withConverter({
  toFirestore(data) => ({ ...data }),
  fromFirestore(snapshot) => new Foo(..),
})

return collectionData(fooCollection); //=> Observable<Foo[]>

I'll address in a future release of Angular/RxFire.

jamesdaniels avatar Sep 08 '21 17:09 jamesdaniels

Would you like to keep this issue open to track here, or should I open another issue in RxFire?

kevin-induro avatar Sep 09 '21 18:09 kevin-induro

This is fine, I have a WIP to add better generics support over there.

jamesdaniels avatar Sep 09 '21 22:09 jamesdaniels

I have been just typecasting manually like so return docData(document) as Observable<AppUser> but I prefer the old way of doing things. Please add better generics support.

ex1tium avatar Sep 22 '21 13:09 ex1tium

@jamesdaniels does this mean we will eventually get functionality in line with the current documentation, or that the documentation will be updated to reflect the new (seemingly downgraded) doc and collection type setting?

Methodician avatar Oct 05 '21 20:10 Methodician

I'm having the same issue. Is there a better solution than casting the data manually?

davidecampello avatar Jan 05 '22 15:01 davidecampello

When using a service I use this:

private userCollection = collection(this.firestore, 'users') as CollectionReference<FirestoreUser | undefined>;

Then: const docRef = doc(this.userCollection, id); seems to type correctly now.

PVermeer avatar Mar 07 '22 13:03 PVermeer