Skip to content

Hide converter from Public API #3794

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Hide converter from Public API #3794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions packages/firestore/exp/src/api/reference.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,15 +317,18 @@ export function addDoc<T>(
firestore._verifyNotTerminated();

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
2 changes: 1 addition & 1 deletion packages/firestore/exp/src/api/snapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export class QuerySnapshot<T = DocumentData> {
doc.key,
doc,
new SnapshotMetadata(hasPendingWrites, fromCache),
this.query.converter
this.query._converter
);
}
}
Expand Down
31 changes: 17 additions & 14 deletions packages/firestore/lite/src/api/reference.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ export class DocumentReference<T = DocumentData> extends _DocumentKeyReference<

constructor(
readonly firestore: FirebaseFirestore,
readonly converter: FirestoreDataConverter<T> | null,
_converter: FirestoreDataConverter<T> | null,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to have its readonly property removed? I don't see the converter value being assigned to a new value anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readonly is already part of the parent class, which already uses the underscore naming.

readonly _path: ResourcePath
) {
super(firestore._databaseId, new DocumentKey(_path), converter);
super(firestore._databaseId, new DocumentKey(_path), _converter);
}

get id(): string {
Expand Down Expand Up @@ -154,7 +154,7 @@ export class Query<T = DocumentData> {
// This is the lite version of the Query class in the main SDK.
constructor(
readonly firestore: FirebaseFirestore,
readonly converter: FirestoreDataConverter<T> | null,
readonly _converter: FirestoreDataConverter<T> | null,
readonly _query: InternalQuery
) {}

Expand Down Expand Up @@ -217,7 +217,7 @@ class QueryFilterConstraint extends QueryConstraint {
);
return new Query(
query.firestore,
query.converter,
query._converter,
queryWithAddedFilter(query._query, filter)
);
}
Expand Down Expand Up @@ -259,7 +259,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 @@ -290,7 +290,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 @@ -324,7 +324,7 @@ class QueryStartAtConstraint extends QueryConstraint {
);
return new Query(
query.firestore,
query.converter,
query._converter,
queryWithStartAt(query._query, bound)
);
}
Expand Down Expand Up @@ -364,7 +364,7 @@ class QueryEndAtConstraint extends QueryConstraint {
);
return new Query(
query.firestore,
query.converter,
query._converter,
queryWithEndAt(query._query, bound)
);
}
Expand Down Expand Up @@ -452,7 +452,7 @@ export class CollectionReference<T = DocumentData> extends Query<T> {
validateNonEmptyArgument('CollectionReference.doc', 'path', path);
const absolutePath = this._path.child(ResourcePath.fromString(path!));
validateDocumentPath(absolutePath);
return new DocumentReference(this.firestore, this.converter, absolutePath);
return new DocumentReference(this.firestore, this._converter, absolutePath);
}

withConverter<U>(
Expand Down Expand Up @@ -578,7 +578,7 @@ export function doc<T>(
validateDocumentPath(absolutePath);
return new DocumentReference(
parent.firestore,
parent instanceof CollectionReference ? parent.converter : null,
parent instanceof CollectionReference ? parent._converter : null,
absolutePath
);
}
Expand Down Expand Up @@ -613,7 +613,7 @@ export function getDocs<T>(query: Query<T>): Promise<QuerySnapshot<T>> {
query.firestore,
doc.key,
doc,
query.converter
query._converter
)
);

Expand Down Expand Up @@ -724,7 +724,10 @@ export function addDoc<T>(
): Promise<DocumentReference<T>> {
const docRef = doc(reference);

const convertedValue = applyFirestoreDataConverter(reference.converter, data);
const convertedValue = applyFirestoreDataConverter(
reference._converter,
data
);

const dataReader = newUserDataReader(reference.firestore);
const parsed = parseSetData(
Expand Down Expand Up @@ -755,7 +758,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 @@ -766,7 +769,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