Add support for collection(coll) and doc(doc) #3403
Conversation
💥 No ChangesetLatest commit: 10f89b2 Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂 If these changes should be published to npm, you need to add a changeset. This PR includes no changesetsWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Click here to learn what changesets are, and how to add one. Click here if you're a maintainer who wants to add a changeset to this PR |
Binary Size ReportAffected SDKs
Test Logs |
| } else { | ||
| const doc = cast(parent, DocumentReference); | ||
| const absolutePath = doc._key.path.child(path); | ||
| if ( |
There was a problem hiding this comment.
When would we hit this error block in TS since the arguments are typed? Or is this for JS users? How could you test that this works properly in TS?
There was a problem hiding this comment.
This would be hit if someone passes a DocumentReference-looking type into our SDK. Our external types only validate that type passed in looks like a DocumentReference (has all the same properties) but they cannot enforce that the provided type is an actual instance of DocumentReference.
| new DocumentKey(absolutePath), | ||
| coll._converter | ||
| parent instanceof CollectionReference ? parent._converter : null |
There was a problem hiding this comment.
Can you add a test to make sure the converter is properly passed/omitted?
This adds new methods, which are pending review in go/firestore-next-amendment.
The new collection methods allows direct access to subcollections. Instead of
collection(doc(coll, 'a'), 'b')we now supportcollection(coll, 'a/b'). This matches the behavior of this method when call withdb(Firestore) already.The same change is made for
doc.