Skip to content
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

Make Converter explicit #4817

Merged
merged 1 commit into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ export class Transaction implements PublicTransaction, Compat<ExpTransaction> {
result._key,
result._document,
result.metadata,
ref._converter
ref.converter
)
)
);
Expand Down Expand Up @@ -775,7 +775,7 @@ export class DocumentReference<T = PublicDocumentData>
result._key,
result._document,
result.metadata,
this._delegate._converter
this._delegate.converter
)
)
);
Expand All @@ -802,7 +802,7 @@ export class DocumentReference<T = PublicDocumentData>
result._key,
result._document,
result.metadata,
this._delegate._converter as UntypedFirestoreDataConverter<T>
this._delegate.converter as UntypedFirestoreDataConverter<T>
)
)
);
Expand Down
2 changes: 2 additions & 0 deletions packages/firestore/src/exp/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
15 changes: 6 additions & 9 deletions packages/firestore/src/exp/reference_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export function getDocFromCache<T>(
doc !== null && doc.hasLocalMutations,
/* fromCache= */ true
),
reference._converter
reference.converter
)
);
}
Expand Down Expand Up @@ -270,7 +270,7 @@ export function setDoc<T>(
const firestore = cast(reference.firestore, FirebaseFirestore);

const convertedValue = applyFirestoreDataConverter(
reference._converter,
reference.converter,
data,
options
);
Expand All @@ -280,7 +280,7 @@ export function setDoc<T>(
'setDoc',
reference._key,
convertedValue,
reference._converter !== null,
reference.converter !== null,
options
);

Expand Down Expand Up @@ -398,18 +398,15 @@ export function addDoc<T>(
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(
dataReader,
'addDoc',
docRef._key,
convertedValue,
reference._converter !== null,
reference.converter !== null,
{}
);

Expand Down Expand Up @@ -794,6 +791,6 @@ function convertToDocSnapshot<T>(
ref._key,
doc,
new SnapshotMetadata(snapshot.hasPendingWrites, snapshot.fromCache),
ref._converter
ref.converter
);
}
6 changes: 3 additions & 3 deletions packages/firestore/src/exp/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ export class QuerySnapshot<T = DocumentData> {
this._snapshot.mutatedKeys.has(doc.key),
this._snapshot.fromCache
),
this.query._converter
this.query.converter
)
);
});
Expand Down Expand Up @@ -497,7 +497,7 @@ export function changesFromSnapshot<T>(
querySnapshot._snapshot.mutatedKeys.has(change.doc.key),
querySnapshot._snapshot.fromCache
),
querySnapshot.query._converter
querySnapshot.query.converter
);
lastDoc = change.doc;
return {
Expand Down Expand Up @@ -525,7 +525,7 @@ export function changesFromSnapshot<T>(
querySnapshot._snapshot.mutatedKeys.has(change.doc.key),
querySnapshot._snapshot.fromCache
),
querySnapshot.query._converter
querySnapshot.query.converter
);
let oldIndex = -1;
let newIndex = -1;
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/exp/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class Transaction extends LiteTransaction {
/* hasPendingWrites= */ false,
/* fromCache= */ false
),
ref._converter
ref.converter
)
);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/firestore/src/lite/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions packages/firestore/src/lite/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class QueryFilterConstraint extends QueryConstraint {
);
return new Query(
query.firestore,
query._converter,
query.converter,
queryWithAddedFilter(query._query, filter)
);
}
Expand Down Expand Up @@ -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)
);
}
Expand Down Expand Up @@ -247,7 +247,7 @@ class QueryLimitConstraint extends QueryConstraint {
_apply<T>(query: Query<T>): Query<T> {
return new Query(
query.firestore,
query._converter,
query.converter,
queryWithLimit(query._query, this._limit, this._limitType)
);
}
Expand Down Expand Up @@ -296,7 +296,7 @@ class QueryStartAtConstraint extends QueryConstraint {
);
return new Query(
query.firestore,
query._converter,
query.converter,
queryWithStartAt(query._query, bound)
);
}
Expand Down Expand Up @@ -378,7 +378,7 @@ class QueryEndAtConstraint extends QueryConstraint {
);
return new Query(
query.firestore,
query._converter,
query.converter,
queryWithEndAt(query._query, bound)
);
}
Expand Down
18 changes: 12 additions & 6 deletions packages/firestore/src/lite/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ export class DocumentReference<T = DocumentData> {
/** @hideconstructor */
constructor(
firestore: FirebaseFirestore,
readonly _converter: FirestoreDataConverter<T> | null,
/**
* If provided, the `FirestoreDataConverter` associated with this instance.
*/
readonly converter: FirestoreDataConverter<T> | null,
readonly _key: DocumentKey
) {
this.firestore = firestore;
Expand Down Expand Up @@ -129,7 +132,7 @@ export class DocumentReference<T = DocumentData> {
get parent(): CollectionReference<T> {
return new CollectionReference<T>(
this.firestore,
this._converter,
this.converter,
this._key.path.popLast()
);
}
Expand Down Expand Up @@ -178,7 +181,10 @@ export class Query<T = DocumentData> {
/** @hideconstructor protected */
constructor(
firestore: FirebaseFirestore,
readonly _converter: FirestoreDataConverter<T> | null,
/**
* If provided, the `FirestoreDataConverter` associated with this instance.
*/
readonly converter: FirestoreDataConverter<T> | null,
readonly _query: InternalQuery
) {
this.firestore = firestore;
Expand Down Expand Up @@ -502,7 +508,7 @@ export function doc<T>(
validateDocumentPath(absolutePath);
return new DocumentReference(
parent.firestore,
parent instanceof CollectionReference ? parent._converter : null,
parent instanceof CollectionReference ? parent.converter : null,
new DocumentKey(absolutePath)
);
}
Expand Down Expand Up @@ -531,7 +537,7 @@ export function refEqual<T>(
return (
left.firestore === right.firestore &&
left.path === right.path &&
left._converter === right._converter
left.converter === right.converter
);
}
return false;
Expand All @@ -554,7 +560,7 @@ export function queryEqual<T>(left: Query<T>, right: Query<T>): boolean {
return (
left.firestore === right.firestore &&
queryEquals(left._query, right._query) &&
left._converter === right._converter
left.converter === right.converter
);
}
return false;
Expand Down
15 changes: 6 additions & 9 deletions packages/firestore/src/lite/reference_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function getDoc<T>(
userDataWriter,
reference._key,
document.isFoundDocument() ? document : null,
reference._converter
reference.converter
);
}
);
Expand Down Expand Up @@ -166,7 +166,7 @@ export function getDocs<T>(query: Query<T>): Promise<QuerySnapshot<T>> {
userDataWriter,
doc.key,
doc,
query._converter
query.converter
)
);

Expand Down Expand Up @@ -227,7 +227,7 @@ export function setDoc<T>(
): Promise<void> {
reference = cast<DocumentReference<T>>(reference, DocumentReference);
const convertedValue = applyFirestoreDataConverter(
reference._converter,
reference.converter,
data,
options
);
Expand All @@ -237,7 +237,7 @@ export function setDoc<T>(
'setDoc',
reference._key,
convertedValue,
reference._converter !== null,
reference.converter !== null,
options
);

Expand Down Expand Up @@ -378,18 +378,15 @@ export function addDoc<T>(
reference = cast<CollectionReference<T>>(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(
dataReader,
'addDoc',
docRef._key,
convertedValue,
docRef._converter !== null,
docRef.converter !== null,
{}
);

Expand Down
8 changes: 4 additions & 4 deletions packages/firestore/src/lite/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@ export class Transaction {
userDataWriter,
doc.key,
doc,
ref._converter
ref.converter
);
} else if (doc.isNoDocument()) {
return new DocumentSnapshot(
this._firestore,
userDataWriter,
ref._key,
null,
ref._converter
ref.converter
);
} else {
throw fail(
Expand Down Expand Up @@ -138,7 +138,7 @@ export class Transaction {
): this {
const ref = validateReference(documentRef, this._firestore);
const convertedValue = applyFirestoreDataConverter(
ref._converter,
ref.converter,
value,
options
);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/src/lite/write_batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class WriteBatch {
const ref = validateReference(documentRef, this._firestore);

const convertedValue = applyFirestoreDataConverter(
ref._converter,
ref.converter,
data,
options
);
Expand All @@ -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()));
Expand Down