Add default converter option in withConverter() (#4577)

firebase · Mar 8, 2021 · b6080a8 · b6080a8
1 parent dccd23d
commit b6080a8
Show file tree

Hide file tree

Showing 7 changed files with 191 additions and 26 deletions.
diff --git a/.changeset/lemon-laws-brush.md b/.changeset/lemon-laws-brush.md
@@ -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)`
diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts
@@ -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>(
@@ -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>;
@@ -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>(

diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts
@@ -286,6 +286,7 @@ export class DocumentReference<T = DocumentData> {
     onCompletion?: () => void
   ): () => void;
 
+  withConverter(converter: null): DocumentReference<DocumentData>;
   withConverter<U>(converter: FirestoreDataConverter<U>): DocumentReference<U>;
 }
 
@@ -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;
@@ -398,6 +399,7 @@ export class Query<T = DocumentData> {
     onCompletion?: () => void
   ): () => void;
 
+  withConverter(converter: null): Query<DocumentData>;
   withConverter<U>(converter: FirestoreDataConverter<U>): Query<U>;
 }
 
@@ -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>;

diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts
@@ -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>)
     );
   }
 }
@@ -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>)
     );
   }
 }
@@ -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>)
     );
   }
 }

diff --git a/packages/firestore/src/lite/reference.ts b/packages/firestore/src/lite/reference.ts
@@ -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);
   }
 }
@@ -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);
   }
 }
@@ -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);
   }

diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts
@@ -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);
@@ -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
@@ -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

diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts
@@ -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);
@@ -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);