angular / angularfire

Angular + Firebase = ❤️
https://firebaseopensource.com/projects/angular/angularfire2
MIT License
7.67k stars 2.19k forks source link

Shadowed type generic in doc; Inherit doc type generic from class #2639

Closed P1X3 closed 3 years ago

P1X3 commented 3 years ago

I think the following line that specifies doc return type should be updated as it is shadowed from Line 41. https://github.com/angular/angularfire/blob/e4400e03e1b81b2717715e6f310fbbb71c586578/src/firestore/collection/collection.ts#L144

Essentially here is what is happening: db.collection<Location>('locations').doc('someid').valueChanges(); // Returns Observable<unknown> db.collection<Location>('locations').doc<Location>('someid').valueChanges(); // Returns Observable<Location>

Ideally, I think this would be more desirable result since type passed to collection call would pass down to doc call: db.collection<Location>('locations').doc('someid').valueChanges(); // Returns Observable<Location> db.collection<Location>('locations').doc<Location>('someid').valueChanges(); // Returns Observable<Location>

If I am not mistaken, updating the code to following will produce more desirable result (IMHO), and provide fallback if any projects out there had different type in collection and doc calls.

doc<T2 = T>(path?: string): AngularFirestoreDocument<T2> { return new AngularFirestoreDocument<T2>(this.ref.doc(path), this.afs); }

Related issues: https://github.com/angular/angularfire/issues/1254 PR that resolved the issue: https://github.com/angular/angularfire/pull/1286

P.S. Pardon formatting. I do not contribute enough, or at all, to know the proper code of conduct, but figured this issue might still be considered for a fix.

P1X3 commented 3 years ago

Here is PR https://github.com/angular/angularfire/pull/2640 in case this is something that needs fixing.

jamesdaniels commented 3 years ago

Should be fixed in 6.1.0-rc.1. Thanks for your help.