-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(firestore): Support Firestore Collection Group Queries #2066
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
Conversation
FYI this has been released for testing on |
This comment has been minimized.
This comment has been minimized.
Adding @davideast for review, alternatively @jhuleatt this could be a good one for a pair review. LMK |
} | ||
``` | ||
|
||
`collectionGroup` returns an `AngularFirestoreCollectionGroup` which is similar to `AngularFirestoreCollection` but as it has no set reference there are no data operation methods such as `add`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this a bit tough to interpret on the first pass.
Maybe something like "... similar to AngularFirestoreCollection
. The main difference is that AngularFirestoreCollectionGroup
has no data operation methods such as add
because it doesn't have a unique(?concrete?) reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like concrete.
* @param queryGroupFn | ||
*/ | ||
collectionGroup<T>(collectionId: string, queryGroupFn?: QueryGroupFn): AngularFirestoreCollectionGroup<T> { | ||
if (major < 6) { throw "collection group queries require Firebase JS SDK >= 6.0"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worth throwing a link to the release notes into this error? https://firebase.google.com/support/release-notes/js#version_600_-_may_7_2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh, probably not. I imagine we'll be talking about so few users over the next couple months & more advanced ones at that.
Thanks for reviewing @jhuleatt |
So awesome <3 I'm needing this, thank you for your feature request @jamesdaniels and @jhuleatt. |
Checklist
yarn install
,yarn test
run successfully? (yes/no; required)Description
Code sample