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

Add default converter option in withConverter() #4577

Merged
merged 4 commits into from
Mar 8, 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
7 changes: 7 additions & 0 deletions .changeset/lemon-laws-brush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'firebase': patch
'@firebase/firestore': patch
'@firebase/firestore-types': patch
---

Added support to remove a FirestoreDataConverter on a Firestore reference by calling `withConverter(null)`
61 changes: 58 additions & 3 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9089,7 +9089,26 @@ declare namespace firebase.firestore {
* provided converter will convert between Firestore data and your custom
* type U.
*
* @param converter Converts objects to and from Firestore.
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @return A DocumentReference<U> that uses the provided converter.
*/
withConverter(converter: null): DocumentReference<DocumentData>;
/**
* Applies a custom data converter to this DocumentReference, allowing you
* to use your own custom model objects with Firestore. When you call
* set(), get(), etc. on the returned DocumentReference instance, the
* provided converter will convert between Firestore data and your custom
* type U.
*
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @return A DocumentReference<U> that uses the provided converter.
*/
withConverter<U>(
Expand Down Expand Up @@ -9544,7 +9563,25 @@ declare namespace firebase.firestore {
* returned Query, the provided converter will convert between Firestore
* data and your custom type U.
*
* @param converter Converts objects to and from Firestore.
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @return A Query<U> that uses the provided converter.
*/
withConverter(converter: null): Query<DocumentData>;
/**
* Applies a custom data converter to this Query, allowing you to use your
* own custom model objects with Firestore. When you call get() on the
* returned Query, the provided converter will convert between Firestore
* data and your custom type U.
*
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @return A Query<U> that uses the provided converter.
*/
withConverter<U>(converter: FirestoreDataConverter<U>): Query<U>;
Expand Down Expand Up @@ -9700,7 +9737,25 @@ declare namespace firebase.firestore {
* on the returned CollectionReference instance, the provided converter will
* convert between Firestore data and your custom type U.
*
* @param converter Converts objects to and from Firestore.
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @return A CollectionReference<U> that uses the provided converter.
*/
withConverter(converter: null): CollectionReference<DocumentData>;
/**
* Applies a custom data converter to this CollectionReference, allowing you
* to use your own custom model objects with Firestore. When you call add()
* on the returned CollectionReference instance, the provided converter will
* convert between Firestore data and your custom type U.
*
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @return A CollectionReference<U> that uses the provided converter.
*/
withConverter<U>(
Expand Down
9 changes: 6 additions & 3 deletions packages/firestore-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ export class DocumentReference<T = DocumentData> {
onCompletion?: () => void
): () => void;

withConverter(converter: null): DocumentReference<DocumentData>;
withConverter<U>(converter: FirestoreDataConverter<U>): DocumentReference<U>;
}

Expand Down Expand Up @@ -315,9 +316,9 @@ export class DocumentSnapshot<T = DocumentData> {
isEqual(other: DocumentSnapshot<T>): boolean;
}

export class QueryDocumentSnapshot<T = DocumentData> extends DocumentSnapshot<
T
> {
export class QueryDocumentSnapshot<
T = DocumentData
> extends DocumentSnapshot<T> {
private constructor();

data(options?: SnapshotOptions): T;
Expand Down Expand Up @@ -398,6 +399,7 @@ export class Query<T = DocumentData> {
onCompletion?: () => void
): () => void;

withConverter(converter: null): Query<DocumentData>;
withConverter<U>(converter: FirestoreDataConverter<U>): Query<U>;
}

Expand Down Expand Up @@ -442,6 +444,7 @@ export class CollectionReference<T = DocumentData> extends Query<T> {

isEqual(other: CollectionReference<T>): boolean;

withConverter(converter: null): CollectionReference<DocumentData>;
withConverter<U>(
converter: FirestoreDataConverter<U>
): CollectionReference<U>;
Expand Down
38 changes: 28 additions & 10 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,14 +830,20 @@ export class DocumentReference<T = PublicDocumentData>
);
}

withConverter(converter: null): PublicDocumentReference<PublicDocumentData>;
withConverter<U>(
converter: PublicFirestoreDataConverter<U>
): PublicDocumentReference<U>;
withConverter<U>(
converter: PublicFirestoreDataConverter<U> | null
): PublicDocumentReference<U> {
return new DocumentReference<U>(
this.firestore,
this._delegate.withConverter(
FirestoreDataConverter.getInstance(this.firestore, converter)
)
converter
? this._delegate.withConverter(
FirestoreDataConverter.getInstance(this.firestore, converter)
)
: (this._delegate.withConverter(null) as ExpDocumentReference<U>)
);
}
}
Expand Down Expand Up @@ -1141,12 +1147,18 @@ export class Query<T = PublicDocumentData>
return onSnapshot(this._delegate, options, observer);
}

withConverter<U>(converter: PublicFirestoreDataConverter<U>): Query<U> {
withConverter(converter: null): Query<PublicDocumentData>;
withConverter<U>(converter: PublicFirestoreDataConverter<U>): Query<U>;
withConverter<U>(
converter: PublicFirestoreDataConverter<U> | null
): Query<U> {
return new Query<U>(
this.firestore,
this._delegate.withConverter(
FirestoreDataConverter.getInstance(this.firestore, converter)
)
converter
? this._delegate.withConverter(
FirestoreDataConverter.getInstance(this.firestore, converter)
)
: (this._delegate.withConverter(null) as ExpQuery<U>)
);
}
}
Expand Down Expand Up @@ -1283,14 +1295,20 @@ export class CollectionReference<T = PublicDocumentData>
return refEqual(this._delegate, other._delegate);
}

withConverter(converter: null): CollectionReference<PublicDocumentData>;
withConverter<U>(
converter: PublicFirestoreDataConverter<U>
): CollectionReference<U>;
withConverter<U>(
converter: PublicFirestoreDataConverter<U> | null
): CollectionReference<U> {
return new CollectionReference<U>(
this.firestore,
this._delegate.withConverter(
FirestoreDataConverter.getInstance(this.firestore, converter)
)
converter
? this._delegate.withConverter(
FirestoreDataConverter.getInstance(this.firestore, converter)
)
: (this._delegate.withConverter(null) as ExpCollectionReference<U>)
);
}
}
Expand Down
20 changes: 17 additions & 3 deletions packages/firestore/src/lite/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,18 @@ export class DocumentReference<T = DocumentData> {
* instance, the provided converter will convert between Firestore data and
* your custom type `U`.
*
* @param converter - Converts objects to and from Firestore.
* Passing in `null` as the converter parameter removes the current
* converter.
*
* @param converter - Converts objects to and from Firestore. Passing in
* `null` removes the current converter.
* @returns A `DocumentReference<U>` that uses the provided converter.
*/
withConverter<U>(converter: FirestoreDataConverter<U>): DocumentReference<U> {
withConverter(converter: null): DocumentReference<DocumentData>;
withConverter<U>(converter: FirestoreDataConverter<U>): DocumentReference<U>;
withConverter<U>(
converter: FirestoreDataConverter<U> | null
): DocumentReference<U> {
return new DocumentReference<U>(this.firestore, converter, this._key);
}
}
Expand Down Expand Up @@ -180,7 +188,9 @@ export class Query<T = DocumentData> {
* @param converter - Converts objects to and from Firestore.
* @returns A `Query<U>` that uses the provided converter.
*/
withConverter<U>(converter: FirestoreDataConverter<U>): Query<U> {
withConverter(converter: null): Query<DocumentData>;
withConverter<U>(converter: FirestoreDataConverter<U>): Query<U>;
withConverter<U>(converter: FirestoreDataConverter<U> | null): Query<U> {
return new Query<U>(this.firestore, converter, this._query);
}
}
Expand Down Expand Up @@ -240,8 +250,12 @@ export class CollectionReference<T = DocumentData> extends Query<T> {
* @param converter - Converts objects to and from Firestore.
* @returns A `CollectionReference<U>` that uses the provided converter.
*/
withConverter(converter: null): CollectionReference<DocumentData>;
withConverter<U>(
converter: FirestoreDataConverter<U>
): CollectionReference<U>;
withConverter<U>(
converter: FirestoreDataConverter<U> | null
): CollectionReference<U> {
return new CollectionReference<U>(this.firestore, converter, this._path);
}
Expand Down
37 changes: 37 additions & 0 deletions packages/firestore/test/integration/api/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,18 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('for DocumentReference.withConverter(null) ', () => {
return withTestDb(persistence, async db => {
const docRef = db
.collection('posts')
.doc()
.withConverter(postConverter)
.withConverter(null);

expect(() => docRef.set(new Post('post', 'author'))).to.throw();
});
});

it('for CollectionReference.withConverter()', () => {
return withTestDb(persistence, async db => {
const coll = db.collection('posts').withConverter(postConverter);
Expand All @@ -1407,6 +1419,17 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('for CollectionReference.withConverter(null)', () => {
return withTestDb(persistence, async db => {
const coll = db
.collection('posts')
.withConverter(postConverter)
.withConverter(null);

expect(() => coll.add(new Post('post', 'author'))).to.throw();
});
});

it('for Query.withConverter()', () => {
return withTestDb(persistence, async db => {
await db
Expand All @@ -1424,6 +1447,20 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('for Query.withConverter(null)', () => {
return withTestDb(persistence, async db => {
await db
.doc('postings/post1')
.set({ title: 'post1', author: 'author1' });
const posts = await db
.collectionGroup('postings')
.withConverter(postConverter)
.withConverter(null)
.get();
expect(posts.docs[0].data()).to.not.be.an.instanceof(Post);
});
});

it('requires the correct converter for Partial usage', async () => {
return withTestDb(persistence, async db => {
const ref = db
Expand Down
45 changes: 38 additions & 7 deletions packages/firestore/test/lite/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,15 @@ describe('withConverter() support', () => {
});
});

it('for DocumentReference.withConverter(null) applies default converter', () => {
return withTestCollection(async coll => {
coll = coll.withConverter(postConverter).withConverter(null);
expect(() =>
setDoc(doc(coll, 'post1'), new Post('post', 'author'))
).to.throw();
});
});

it('for CollectionReference.withConverter()', () => {
return withTestCollection(async coll => {
coll = coll.withConverter(postConverter);
Expand All @@ -1125,16 +1134,38 @@ describe('withConverter() support', () => {
});
});

it('for Query.withConverter()', () => {
return withTestCollection(async coll => {
coll = coll.withConverter(postConverter);
await setDoc(doc(coll, 'post1'), new Post('post1', 'author1'));
const posts = await getDocs(coll);
expect(posts.size).to.equal(1);
expect(posts.docs[0].data()!.byline()).to.equal('post1, by author1');
it('for CollectionReference.withConverter(null) applies default converter', () => {
return withTestDoc(async doc => {
doc = doc.withConverter(postConverter).withConverter(null);
expect(() => setDoc(doc, new Post('post', 'author'))).to.throw();
});
});

it('for Query.withConverter()', () => {
return withTestCollectionAndInitialData(
[{ title: 'post', author: 'author' }],
async collRef => {
let query1 = query(collRef, where('title', '==', 'post'));
query1 = query1.withConverter(postConverter);
const result = await getDocs(query1);
expect(result.docs[0].data()).to.be.an.instanceOf(Post);
expect(result.docs[0].data()!.byline()).to.equal('post, by author');
}
);
});

it('for Query.withConverter(null) applies default converter', () => {
return withTestCollectionAndInitialData(
[{ title: 'post', author: 'author' }],
async collRef => {
let query1 = query(collRef, where('title', '==', 'post'));
query1 = query1.withConverter(postConverter).withConverter(null);
const result = await getDocs(query1);
expect(result.docs[0]).to.not.be.an.instanceOf(Post);
}
);
});

it('keeps the converter when calling parent() with a DocumentReference', () => {
return withTestDb(async db => {
const coll = doc(db, 'root/doc').withConverter(postConverter);
Expand Down