Add getDoc() to firestore-exp #3274
Conversation
schmidt-sebastian
force-pushed
the
mrschmidt/getdoc-exp
branch
4 times, most recently
from
d6a126c to
a970170
Compare
🦋 Changeset is good to goLatest commit: 11bc080 We got this. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| export { | ||
| DocumentReference, | ||
| CollectionReference, | ||
| Query, | ||
| doc, | ||
| collection, | ||
| collectionGroup, | ||
| parent | ||
| } from '../lite/src/api/reference'; | ||
|
|
||
| export { runTransaction, Transaction } from '../lite/src/api/transaction'; |
There was a problem hiding this comment.
All these exports can be re-used without modification since they don't rely on latency-compensation.
Binary Size ReportAffected SDKs
Test Logs |
schmidt-sebastian
force-pushed
the
mrschmidt/getdoc-exp
branch
from
a970170 to
64162a2
Compare
schmidt-sebastian
force-pushed
the
mrschmidt/getdoc-exp
branch
from
08bb538 to
64162a2
Compare
| private _convertToDocSnapshot(snapshot: ViewSnapshot): DocumentSnapshot<T> { | ||
| debugAssert( | ||
| snapshot.docs.size <= 1, | ||
| 'Too many documents returned on a document query' |
There was a problem hiding this comment.
This message is odd relative to the check being done. Can you add a comment with some context?
There was a problem hiding this comment.
Changed to Expected zero or a single result on a document-only query.
| @@ -41,6 +41,7 @@ import { Settings } from '../../'; | |||
| // settings() defaults: | |||
| const DEFAULT_HOST = 'firestore.googleapis.com'; | |||
| const DEFAULT_SSL = true; | |||
| const DEFAULT_FORCE_LONG_POLLING = false; // Used by full SDK | |||
There was a problem hiding this comment.
Is this necessary given the fixes we have in the pipeline? Will you pull this out later?
There was a problem hiding this comment.
The current API of the experimental SDK exposes this setting. We can probably remove it once we update our closure dependency (which, by the way, seems to be unblocked now: https://www.npmjs.com/package/google-closure-library). With that update, we could just turn on the new mode in the experimental build directly and not expose it as an option.
schmidt-sebastian
requested review from
Feiyang1,
hiranya911 and
hsubox76
as code owners
This adds getDoc() to the firestore-exp SDK. It re-uses most code from the Lite SDK where possible (e.g. DocumentReferences, CollectionReferences) but includes new full featured implementation for DocumentSnapshot and QueryDocumentSnapshot (tests for those will come when we add setDoc).
The new
getDoc()uses FirestoreClient to fetch a latency compensated snapshot, and uses some factored out methods from the current SDK to convert them to a ViewSnapshot. This is the reason for the changes insrc/api/database.ts, which is refactored so that the existing methods to use generic ViewSnapshots (which can be consumed by both the current and the firestore-exp SDK).