Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
P1X3 opened this issue Nov 9, 2020 · 2 comments
Closed

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

P1X3 opened this issue Nov 9, 2020 · 2 comments

Comments

@P1X3
Copy link
Contributor

P1X3 commented Nov 9, 2020

I think the following line that specifies doc return type should be updated as it is shadowed from Line 41.

doc<T>(path?: string): AngularFirestoreDocument<T> {

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: #1254
PR that resolved the issue: #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
Copy link
Contributor Author

P1X3 commented Nov 9, 2020

Here is PR #2640 in case this is something that needs fixing.

@jamesdaniels
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants