From ba2b11fe312889c5fb2b46d76804164130f2955e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Apr 2021 11:52:08 -0600 Subject: [PATCH] Make Converter explicit (#4817) --- packages/firestore/src/api/database.ts | 6 +++--- packages/firestore/src/exp/database.ts | 2 ++ packages/firestore/src/exp/reference_impl.ts | 15 ++++++--------- packages/firestore/src/exp/snapshot.ts | 6 +++--- packages/firestore/src/exp/transaction.ts | 2 +- packages/firestore/src/lite/database.ts | 2 ++ packages/firestore/src/lite/query.ts | 10 +++++----- packages/firestore/src/lite/reference.ts | 18 ++++++++++++------ packages/firestore/src/lite/reference_impl.ts | 15 ++++++--------- packages/firestore/src/lite/transaction.ts | 8 ++++---- packages/firestore/src/lite/write_batch.ts | 4 ++-- 11 files changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index d3002c2ad51..e7e571ce21f 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -409,7 +409,7 @@ export class Transaction implements PublicTransaction, Compat { result._key, result._document, result.metadata, - ref._converter + ref.converter ) ) ); @@ -775,7 +775,7 @@ export class DocumentReference result._key, result._document, result.metadata, - this._delegate._converter + this._delegate.converter ) ) ); @@ -802,7 +802,7 @@ export class DocumentReference result._key, result._document, result.metadata, - this._delegate._converter as UntypedFirestoreDataConverter + this._delegate.converter as UntypedFirestoreDataConverter ) ) ); diff --git a/packages/firestore/src/exp/database.ts b/packages/firestore/src/exp/database.ts index c79f23a28f0..76df6a72e9f 100644 --- a/packages/firestore/src/exp/database.ts +++ b/packages/firestore/src/exp/database.ts @@ -80,6 +80,8 @@ export const CACHE_SIZE_UNLIMITED = LRU_COLLECTION_DISABLED; * Do not call this constructor directly. Instead, use {@link getFirestore}. */ export class FirebaseFirestore extends LiteFirestore { + type: 'firestore-lite' | 'firestore' = 'firestore'; + readonly _queue: AsyncQueue = newAsyncQueue(); readonly _persistenceKey: string; diff --git a/packages/firestore/src/exp/reference_impl.ts b/packages/firestore/src/exp/reference_impl.ts index ec41ed73dd9..0eee61c267d 100644 --- a/packages/firestore/src/exp/reference_impl.ts +++ b/packages/firestore/src/exp/reference_impl.ts @@ -143,7 +143,7 @@ export function getDocFromCache( doc !== null && doc.hasLocalMutations, /* fromCache= */ true ), - reference._converter + reference.converter ) ); } @@ -270,7 +270,7 @@ export function setDoc( const firestore = cast(reference.firestore, FirebaseFirestore); const convertedValue = applyFirestoreDataConverter( - reference._converter, + reference.converter, data, options ); @@ -280,7 +280,7 @@ export function setDoc( 'setDoc', reference._key, convertedValue, - reference._converter !== null, + reference.converter !== null, options ); @@ -398,10 +398,7 @@ export function addDoc( const firestore = cast(reference.firestore, FirebaseFirestore); const docRef = doc(reference); - const convertedValue = applyFirestoreDataConverter( - reference._converter, - data - ); + const convertedValue = applyFirestoreDataConverter(reference.converter, data); const dataReader = newUserDataReader(reference.firestore); const parsed = parseSetData( @@ -409,7 +406,7 @@ export function addDoc( 'addDoc', docRef._key, convertedValue, - reference._converter !== null, + reference.converter !== null, {} ); @@ -794,6 +791,6 @@ function convertToDocSnapshot( ref._key, doc, new SnapshotMetadata(snapshot.hasPendingWrites, snapshot.fromCache), - ref._converter + ref.converter ); } diff --git a/packages/firestore/src/exp/snapshot.ts b/packages/firestore/src/exp/snapshot.ts index f09994a9301..c7a1f499527 100644 --- a/packages/firestore/src/exp/snapshot.ts +++ b/packages/firestore/src/exp/snapshot.ts @@ -427,7 +427,7 @@ export class QuerySnapshot { this._snapshot.mutatedKeys.has(doc.key), this._snapshot.fromCache ), - this.query._converter + this.query.converter ) ); }); @@ -497,7 +497,7 @@ export function changesFromSnapshot( querySnapshot._snapshot.mutatedKeys.has(change.doc.key), querySnapshot._snapshot.fromCache ), - querySnapshot.query._converter + querySnapshot.query.converter ); lastDoc = change.doc; return { @@ -525,7 +525,7 @@ export function changesFromSnapshot( querySnapshot._snapshot.mutatedKeys.has(change.doc.key), querySnapshot._snapshot.fromCache ), - querySnapshot.query._converter + querySnapshot.query.converter ); let oldIndex = -1; let newIndex = -1; diff --git a/packages/firestore/src/exp/transaction.ts b/packages/firestore/src/exp/transaction.ts index c1790b472fd..f8374f479b0 100644 --- a/packages/firestore/src/exp/transaction.ts +++ b/packages/firestore/src/exp/transaction.ts @@ -66,7 +66,7 @@ export class Transaction extends LiteTransaction { /* hasPendingWrites= */ false, /* fromCache= */ false ), - ref._converter + ref.converter ) ); } diff --git a/packages/firestore/src/lite/database.ts b/packages/firestore/src/lite/database.ts index 4d4ef42e6c9..6b510530d6b 100644 --- a/packages/firestore/src/lite/database.ts +++ b/packages/firestore/src/lite/database.ts @@ -56,6 +56,8 @@ declare module '@firebase/component' { * Do not call this constructor directly. Instead, use {@link getFirestore}. */ export class FirebaseFirestore implements FirestoreService { + type: 'firestore-lite' | 'firestore' = 'firestore-lite'; + readonly _databaseId: DatabaseId; readonly _persistenceKey: string = '(lite)'; _credentials: CredentialsProvider; diff --git a/packages/firestore/src/lite/query.ts b/packages/firestore/src/lite/query.ts index c7cdb576412..b0bc324a25d 100644 --- a/packages/firestore/src/lite/query.ts +++ b/packages/firestore/src/lite/query.ts @@ -147,7 +147,7 @@ class QueryFilterConstraint extends QueryConstraint { ); return new Query( query.firestore, - query._converter, + query.converter, queryWithAddedFilter(query._query, filter) ); } @@ -205,7 +205,7 @@ class QueryOrderByConstraint extends QueryConstraint { const orderBy = newQueryOrderBy(query._query, this._field, this._direction); return new Query( query.firestore, - query._converter, + query.converter, queryWithAddedOrderBy(query._query, orderBy) ); } @@ -247,7 +247,7 @@ class QueryLimitConstraint extends QueryConstraint { _apply(query: Query): Query { return new Query( query.firestore, - query._converter, + query.converter, queryWithLimit(query._query, this._limit, this._limitType) ); } @@ -296,7 +296,7 @@ class QueryStartAtConstraint extends QueryConstraint { ); return new Query( query.firestore, - query._converter, + query.converter, queryWithStartAt(query._query, bound) ); } @@ -378,7 +378,7 @@ class QueryEndAtConstraint extends QueryConstraint { ); return new Query( query.firestore, - query._converter, + query.converter, queryWithEndAt(query._query, bound) ); } diff --git a/packages/firestore/src/lite/reference.ts b/packages/firestore/src/lite/reference.ts index e97c9eebc7b..44933301a02 100644 --- a/packages/firestore/src/lite/reference.ts +++ b/packages/firestore/src/lite/reference.ts @@ -98,7 +98,10 @@ export class DocumentReference { /** @hideconstructor */ constructor( firestore: FirebaseFirestore, - readonly _converter: FirestoreDataConverter | null, + /** + * If provided, the `FirestoreDataConverter` associated with this instance. + */ + readonly converter: FirestoreDataConverter | null, readonly _key: DocumentKey ) { this.firestore = firestore; @@ -129,7 +132,7 @@ export class DocumentReference { get parent(): CollectionReference { return new CollectionReference( this.firestore, - this._converter, + this.converter, this._key.path.popLast() ); } @@ -178,7 +181,10 @@ export class Query { /** @hideconstructor protected */ constructor( firestore: FirebaseFirestore, - readonly _converter: FirestoreDataConverter | null, + /** + * If provided, the `FirestoreDataConverter` associated with this instance. + */ + readonly converter: FirestoreDataConverter | null, readonly _query: InternalQuery ) { this.firestore = firestore; @@ -502,7 +508,7 @@ export function doc( validateDocumentPath(absolutePath); return new DocumentReference( parent.firestore, - parent instanceof CollectionReference ? parent._converter : null, + parent instanceof CollectionReference ? parent.converter : null, new DocumentKey(absolutePath) ); } @@ -531,7 +537,7 @@ export function refEqual( return ( left.firestore === right.firestore && left.path === right.path && - left._converter === right._converter + left.converter === right.converter ); } return false; @@ -554,7 +560,7 @@ export function queryEqual(left: Query, right: Query): boolean { return ( left.firestore === right.firestore && queryEquals(left._query, right._query) && - left._converter === right._converter + left.converter === right.converter ); } return false; diff --git a/packages/firestore/src/lite/reference_impl.ts b/packages/firestore/src/lite/reference_impl.ts index 9385b0de363..91c59bbb1b3 100644 --- a/packages/firestore/src/lite/reference_impl.ts +++ b/packages/firestore/src/lite/reference_impl.ts @@ -134,7 +134,7 @@ export function getDoc( userDataWriter, reference._key, document.isFoundDocument() ? document : null, - reference._converter + reference.converter ); } ); @@ -166,7 +166,7 @@ export function getDocs(query: Query): Promise> { userDataWriter, doc.key, doc, - query._converter + query.converter ) ); @@ -227,7 +227,7 @@ export function setDoc( ): Promise { reference = cast>(reference, DocumentReference); const convertedValue = applyFirestoreDataConverter( - reference._converter, + reference.converter, data, options ); @@ -237,7 +237,7 @@ export function setDoc( 'setDoc', reference._key, convertedValue, - reference._converter !== null, + reference.converter !== null, options ); @@ -378,10 +378,7 @@ export function addDoc( reference = cast>(reference, CollectionReference); const docRef = doc(reference); - const convertedValue = applyFirestoreDataConverter( - reference._converter, - data - ); + const convertedValue = applyFirestoreDataConverter(reference.converter, data); const dataReader = newUserDataReader(reference.firestore); const parsed = parseSetData( @@ -389,7 +386,7 @@ export function addDoc( 'addDoc', docRef._key, convertedValue, - docRef._converter !== null, + docRef.converter !== null, {} ); diff --git a/packages/firestore/src/lite/transaction.ts b/packages/firestore/src/lite/transaction.ts index e1182410e51..24d4dbc18c5 100644 --- a/packages/firestore/src/lite/transaction.ts +++ b/packages/firestore/src/lite/transaction.ts @@ -88,7 +88,7 @@ export class Transaction { userDataWriter, doc.key, doc, - ref._converter + ref.converter ); } else if (doc.isNoDocument()) { return new DocumentSnapshot( @@ -96,7 +96,7 @@ export class Transaction { userDataWriter, ref._key, null, - ref._converter + ref.converter ); } else { throw fail( @@ -138,7 +138,7 @@ export class Transaction { ): this { const ref = validateReference(documentRef, this._firestore); const convertedValue = applyFirestoreDataConverter( - ref._converter, + ref.converter, value, options ); @@ -147,7 +147,7 @@ export class Transaction { 'Transaction.set', ref._key, convertedValue, - ref._converter !== null, + ref.converter !== null, options ); this._transaction.set(ref._key, parsed); diff --git a/packages/firestore/src/lite/write_batch.ts b/packages/firestore/src/lite/write_batch.ts index a60c6a79d00..b3e8f681d76 100644 --- a/packages/firestore/src/lite/write_batch.ts +++ b/packages/firestore/src/lite/write_batch.ts @@ -93,7 +93,7 @@ export class WriteBatch { const ref = validateReference(documentRef, this._firestore); const convertedValue = applyFirestoreDataConverter( - ref._converter, + ref.converter, data, options ); @@ -102,7 +102,7 @@ export class WriteBatch { 'WriteBatch.set', ref._key, convertedValue, - ref._converter !== null, + ref.converter !== null, options ); this._mutations.push(parsed.toMutation(ref._key, Precondition.none()));