Closed nathanosdev closed 3 years ago
can u make PR?
I could make a PR over the weekend, but it would be nice to have confirmation from the author's of the library that they will support such a change.
This issue is also related to these request: #2361 and #2438 .
At the end, the types need some fix to provide the correct generics. I think I could submit a PR, need to checkout what's need.
UPDATE: I don't think changing the angularfire functions with the proper return type would solve this as the underlying types from the firebase-js-sdk
are not generic as well. Please see packages/firestore-types/index.d.ts and notice how most of them have the DocumentData instead of generics.
Hello angularFire team. I literally spend a ton of money in your app for my clients. At least answer us. It’s a shame that core features are broken. It’s a shame you don’t manage the PR. At least give us back the projets lead and we can manage it like a TRUE open source project.
@KingDarBoja thanks for checking out the Firestore
types. I understand your concern, but after looking into this a bit deeper I'm confident that it will work.
For https://github.com/angular/angularfire/issues/2361 we need to make the change here in the constructor: https://github.com/angular/angularfire/blob/7cb6c03481af7a7cf2063b1fcc6b37b1c657221b/src/firestore/document/document.ts#L37
We can take the generic parameter from the AngularFirestoreDocument
class and give it to the DocumentReference
type in the constructor.
constructor(public ref: DocumentReference<T>, private afs: AngularFirestore) { }
The DocumentReference.get
method in question will then use that generic type.
get(options?: GetOptions): Promise<DocumentSnapshot<T>>;
so then the AngularFirestoreDocument.get
method will be able to correctly infer the generic return type.
https://github.com/angular/angularfire/blob/7cb6c03481af7a7cf2063b1fcc6b37b1c657221b/src/firestore/document/document.ts#L95
And for the other issue you linked: https://github.com/angular/angularfire/issues/2438
The Query
type from Firestore is generic: https://github.com/firebase/firebase-js-sdk/blob/9a9a81fe4f001f23e9fe1db054c2e7159fca3ae3/packages/firestore-types/index.d.ts#L302
So similar to the previous issue, we can provide a generic type to that type in the constructor: https://github.com/angular/angularfire/blob/c57cef4a0d08f7df43754a30c18d707d1806cbd0/src/firestore/collection/collection.ts#L53
private readonly query: Query<T>,
and then when we call AngularFirestoreCollection.get
it will be able to correctly infer the return type.
https://github.com/angular/angularfire/blob/c57cef4a0d08f7df43754a30c18d707d1806cbd0/src/firestore/collection/collection.ts#L125
@BianchiSeb I couldn't agree more. I've done the research into how to achieve this and the 2 other issues that have been linked here. I am ready to make a PR for this, but looking at the history of issues and PRs, it does not look like my PR would go anywhere, which is very disheartening.
@animamundi Thanks for sharing the solution to this issue, I remember trying to fix it by providing the generics at the interfaces.ts
definition but always got an error regarding T
being same name but probably being different type.
Anyway, I suggest to submit the PR and the community should bring support to get it merged (thumbs up on the topic), it is better than nothing.
Should be addressed in 6.1.0-rc.1
The
DocumentReference
type is wrapping the same type fromfirestore
in a way that hides the generic type.I would suggest also proxying the generic parameter like so:
I didn't check everywhere that this model is used, but usages would also need to be adjusted. For example:
This would allow for much better type safety when working with the
DocumentReference
model.