Make Converter passing explicit (#3171)

Author: schmidt-sebastian

packages/firestore/lite/src/api/reference.ts

@@ -55,7 +55,7 @@ export class DocumentReference<T = firestore.DocumentData>
   constructor(
     readonly firestore: Firestore,
     key: DocumentKey,
-    readonly _converter?: firestore.FirestoreDataConverter<T>
+    readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {
     super(firestore._databaseId, key, _converter);
   }
@@ -79,7 +79,7 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
   constructor(
     readonly firestore: Firestore,
     readonly _query: InternalQuery,
-    readonly _converter?: firestore.FirestoreDataConverter<T>
+    readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {}
 
   where(
@@ -153,7 +153,7 @@ export class CollectionReference<T = firestore.DocumentData> extends Query<T>
   constructor(
     readonly firestore: Firestore,
     readonly _path: ResourcePath,
-    readonly _converter?: firestore.FirestoreDataConverter<T>
+    readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {
     super(firestore, InternalQuery.atPath(_path), _converter);
   }
@@ -189,12 +189,16 @@ export function collection(
   const path = ResourcePath.fromString(relativePath);
   if (parent instanceof Firestore) {
     validateCollectionPath(path);
-    return new CollectionReference(parent, path);
+    return new CollectionReference(parent, path, /* converter= */ null);
   } else {
     const doc = cast(parent, DocumentReference);
     const absolutePath = doc._key.path.child(path);
     validateCollectionPath(absolutePath);
-    return new CollectionReference(doc.firestore, absolutePath);
+    return new CollectionReference(
+      doc.firestore,
+      absolutePath,
+      /* converter= */ null
+    );
   }
 }
 
@@ -219,7 +223,11 @@ export function doc<T>(
   const path = ResourcePath.fromString(relativePath!);
   if (parent instanceof Firestore) {
     validateDocumentPath(path);
-    return new DocumentReference(parent, new DocumentKey(path));
+    return new DocumentReference(
+      parent,
+      new DocumentKey(path),
+      /* converter= */ null
+    );
   } else {
     const coll = cast(parent, CollectionReference);
     const absolutePath = coll._path.child(path);
@@ -248,7 +256,8 @@ export function parent<T>(
     } else {
       return new DocumentReference(
         child.firestore,
-        new DocumentKey(parentPath)
+        new DocumentKey(parentPath),
+        /* converter= */ null
       );
     }
   } else {

packages/firestore/lite/src/api/snapshot.ts

@@ -38,7 +38,7 @@ export class DocumentSnapshot<T = firestore.DocumentData>
     private _firestore: Firestore,
     private _key: DocumentKey,
     private _document: Document | null,
-    private _converter?: firestore.FirestoreDataConverter<T>
+    private _converter: firestore.FirestoreDataConverter<T> | null
   ) {}
 
   get id(): string {
@@ -66,15 +66,17 @@ export class DocumentSnapshot<T = firestore.DocumentData>
       const snapshot = new QueryDocumentSnapshot(
         this._firestore,
         this._key,
-        this._document
+        this._document,
+        /* converter= */ null
       );
       return this._converter.fromFirestore(snapshot);
     } else {
       const userDataWriter = new UserDataWriter(
         this._firestore._databaseId,
         /* timestampsInSnapshots= */ true,
         /* serverTimestampBehavior=*/ 'none',
-        key => new DocumentReference(this._firestore, key)
+        key =>
+          new DocumentReference(this._firestore, key, /* converter= */ null)
       );
       return userDataWriter.convertValue(this._document.toProto()) as T;
     }

packages/firestore/src/api/database.ts

@@ -580,14 +580,22 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
     validateExactNumberOfArgs('Firestore.collection', arguments, 1);
     validateArgType('Firestore.collection', 'non-empty string', 1, pathString);
     this.ensureClientConfigured();
-    return new CollectionReference(ResourcePath.fromString(pathString), this);
+    return new CollectionReference(
+      ResourcePath.fromString(pathString),
+      this,
+      /* converter= */ null
+    );
   }
 
   doc(pathString: string): firestore.DocumentReference {
     validateExactNumberOfArgs('Firestore.doc', arguments, 1);
     validateArgType('Firestore.doc', 'non-empty string', 1, pathString);
     this.ensureClientConfigured();
-    return DocumentReference.forPath(ResourcePath.fromString(pathString), this);
+    return DocumentReference.forPath(
+      ResourcePath.fromString(pathString),
+      this,
+      /* converter= */ null
+    );
   }
 
   collectionGroup(collectionId: string): firestore.Query {
@@ -608,7 +616,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
     this.ensureClientConfigured();
     return new Query(
       new InternalQuery(ResourcePath.EMPTY_PATH, collectionId),
-      this
+      this,
+      /* converter= */ null
     );
   }
 
@@ -946,7 +955,7 @@ export class DocumentReference<T = firestore.DocumentData>
   constructor(
     public _key: DocumentKey,
     readonly firestore: Firestore,
-    readonly _converter?: firestore.FirestoreDataConverter<T>
+    readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {
     super(firestore._databaseId, _key, _converter);
     this._firestoreClient = this.firestore.ensureClientConfigured();
@@ -955,7 +964,7 @@ export class DocumentReference<T = firestore.DocumentData>
   static forPath<U>(
     path: ResourcePath,
     firestore: Firestore,
-    converter?: firestore.FirestoreDataConverter<U>
+    converter: firestore.FirestoreDataConverter<U> | null
   ): DocumentReference<U> {
     if (path.length % 2 !== 0) {
       throw new FirestoreError(
@@ -1001,7 +1010,11 @@ export class DocumentReference<T = firestore.DocumentData>
       );
     }
     const path = ResourcePath.fromString(pathString);
-    return new CollectionReference(this._key.path.child(path), this.firestore);
+    return new CollectionReference(
+      this._key.path.child(path),
+      this.firestore,
+      /* converter= */ null
+    );
   }
 
   isEqual(other: firestore.DocumentReference<T>): boolean {
@@ -1328,7 +1341,7 @@ export class DocumentSnapshot<T = firestore.DocumentData>
     public _document: Document | null,
     private _fromCache: boolean,
     private _hasPendingWrites: boolean,
-    private readonly _converter?: firestore.FirestoreDataConverter<T>
+    private readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {}
 
   data(options?: firestore.SnapshotOptions): T | undefined {
@@ -1345,15 +1358,17 @@ export class DocumentSnapshot<T = firestore.DocumentData>
           this._key,
           this._document,
           this._fromCache,
-          this._hasPendingWrites
+          this._hasPendingWrites,
+          /* converter= */ null
         );
         return this._converter.fromFirestore(snapshot, options);
       } else {
         const userDataWriter = new UserDataWriter(
           this._firestore._databaseId,
           this._firestore._areTimestampsInSnapshotsEnabled(),
           options.serverTimestamps || 'none',
-          key => new DocumentReference(key, this._firestore)
+          key =>
+            new DocumentReference(key, this._firestore, /* converter= */ null)
         );
         return userDataWriter.convertValue(this._document.toProto()) as T;
       }
@@ -1436,7 +1451,7 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
   constructor(
     public _query: InternalQuery,
     readonly firestore: Firestore,
-    protected readonly _converter?: firestore.FirestoreDataConverter<T>
+    protected readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {}
 
   where(
@@ -2170,7 +2185,7 @@ export class QuerySnapshot<T = firestore.DocumentData>
     private readonly _firestore: Firestore,
     private readonly _originalQuery: InternalQuery,
     private readonly _snapshot: ViewSnapshot,
-    private readonly _converter?: firestore.FirestoreDataConverter<T>
+    private readonly _converter: firestore.FirestoreDataConverter<T> | null
   ) {
     this.metadata = new SnapshotMetadata(
       _snapshot.hasPendingWrites,
@@ -2281,7 +2296,7 @@ export class CollectionReference<T = firestore.DocumentData> extends Query<T>
   constructor(
     readonly _path: ResourcePath,
     firestore: Firestore,
-    _converter?: firestore.FirestoreDataConverter<T>
+    _converter: firestore.FirestoreDataConverter<T> | null
   ) {
     super(InternalQuery.atPath(_path), firestore, _converter);
     if (_path.length % 2 !== 1) {
@@ -2305,7 +2320,8 @@ export class CollectionReference<T = firestore.DocumentData> extends Query<T>
     } else {
       return new DocumentReference<firestore.DocumentData>(
         new DocumentKey(parentPath),
-        this.firestore
+        this.firestore,
+        /* converter= */ null
       );
     }
   }
@@ -2446,7 +2462,7 @@ export function changesFromSnapshot<T>(
   firestore: Firestore,
   includeMetadataChanges: boolean,
   snapshot: ViewSnapshot,
-  converter?: firestore.FirestoreDataConverter<T>
+  converter: firestore.FirestoreDataConverter<T> | null
 ): Array<firestore.DocumentChange<T>> {
   if (snapshot.oldDocs.isEmpty()) {
     // Special case the first snapshot because index calculation is easy and
@@ -2535,7 +2551,7 @@ function resultChangeType(type: ChangeType): firestore.DocumentChangeType {
  * call.
  */
 export function applyFirestoreDataConverter<T>(
-  converter: UntypedFirestoreDataConverter<T> | undefined,
+  converter: UntypedFirestoreDataConverter<T> | null,
   value: T,
   functionName: string
 ): [firestore.DocumentData, string] {

packages/firestore/src/api/user_data_reader.ts

@@ -65,7 +65,7 @@ export class DocumentKeyReference<T> {
   constructor(
     public readonly _databaseId: DatabaseId,
     public readonly _key: DocumentKey,
-    public readonly _converter?: UntypedFirestoreDataConverter<T>
+    public readonly _converter: UntypedFirestoreDataConverter<T> | null
   ) {}
 }
 

packages/firestore/test/unit/api/document_change.test.ts

@@ -53,7 +53,12 @@ describe('DocumentChange:', () => {
     const expected = documentSetAsArray(updatedSnapshot.docs);
     const actual = documentSetAsArray(initialSnapshot.docs);
 
-    const changes = changesFromSnapshot({} as Firestore, true, updatedSnapshot);
+    const changes = changesFromSnapshot(
+      {} as Firestore,
+      true,
+      updatedSnapshot,
+      /* converter= */ null
+    );
 
     for (const change of changes) {
       if (change.type !== 'added') {

packages/firestore/test/util/api_helpers.ts

@@ -45,8 +45,8 @@ import { Provider, ComponentContainer } from '@firebase/component';
  */
 export const FIRESTORE = new Firestore(
   {
-    projectId: 'projectid',
-    database: 'database'
+    projectId: 'test-project',
+    database: '(default)'
   },
   new Provider('auth-internal', new ComponentContainer('default')),
   new IndexedDbComponentProvider()
@@ -57,11 +57,15 @@ export function firestore(): Firestore {
 }
 
 export function collectionReference(path: string): CollectionReference {
-  return new CollectionReference(pathFrom(path), firestore());
+  return new CollectionReference(
+    pathFrom(path),
+    firestore(),
+    /* converter= */ null
+  );
 }
 
 export function documentReference(path: string): DocumentReference {
-  return new DocumentReference(key(path), firestore());
+  return new DocumentReference(key(path), firestore(), /* converter= */ null);
 }
 
 export function documentSnapshot(
@@ -75,21 +79,27 @@ export function documentSnapshot(
       key(path),
       doc(path, 1, data),
       fromCache,
-      /* hasPendingWrites= */ false
+      /* hasPendingWrites= */ false,
+      /* converter= */ null
     );
   } else {
     return new DocumentSnapshot(
       firestore(),
       key(path),
       null,
       fromCache,
-      /* hasPendingWrites= */ false
+      /* hasPendingWrites= */ false,
+      /* converter= */ null
     );
   }
 }
 
 export function query(path: string): Query {
-  return new Query(InternalQuery.atPath(pathFrom(path)), firestore());
+  return new Query(
+    InternalQuery.atPath(pathFrom(path)),
+    firestore(),
+    /* converter= */ null
+  );
 }
 
 /**
@@ -135,5 +145,10 @@ export function querySnapshot(
     syncStateChanged,
     false
   );
-  return new QuerySnapshot(firestore(), query, viewSnapshot);
+  return new QuerySnapshot(
+    firestore(),
+    query,
+    viewSnapshot,
+    /* converter= */ null
+  );
 }

packages/firestore/test/util/helpers.ts

@@ -84,32 +84,25 @@ import { primitiveComparator } from '../../src/util/misc';
 import { Dict, forEach } from '../../src/util/obj';
 import { SortedMap } from '../../src/util/sorted_map';
 import { SortedSet } from '../../src/util/sorted_set';
-import { query } from './api_helpers';
+import { FIRESTORE, query } from './api_helpers';
 import { ByteString } from '../../src/util/byte_string';
 import { PlatformSupport } from '../../src/platform/platform';
 import { JsonProtoSerializer } from '../../src/remote/serializer';
 import { Timestamp } from '../../src/api/timestamp';
-import { DocumentReference, Firestore } from '../../src/api/database';
+import { DocumentReference } from '../../src/api/database';
 import { DeleteFieldValueImpl } from '../../src/api/field_value';
 import { Code, FirestoreError } from '../../src/util/error';
 
 /* eslint-disable no-restricted-globals */
 
-// A Firestore that can be used in DocumentReferences and UserDataWriter.
-const fakeFirestore: Firestore = {
-  ensureClientConfigured: () => {},
-  _databaseId: new DatabaseId('test-project')
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-} as any;
-
 export type TestSnapshotVersion = number;
 
 export function testUserDataWriter(): UserDataWriter {
   return new UserDataWriter(
     new DatabaseId('test-project'),
     /* timestampsInSnapshots= */ false,
     'none',
-    key => new DocumentReference(key, fakeFirestore)
+    key => new DocumentReference(key, FIRESTORE, /* converter= */ null)
   );
 }
 
@@ -133,7 +126,8 @@ export function version(v: TestSnapshotVersion): SnapshotVersion {
 export function ref(key: string, offset?: number): DocumentReference {
   return new DocumentReference(
     new DocumentKey(path(key, offset)),
-    fakeFirestore
+    FIRESTORE,
+    /* converter= */ null
   );
 }