feat(types): deprecate serializer in favor of converter

Author: posva

src/firestore/index.ts

@@ -1,4 +1,9 @@
-import { createSnapshot, extractRefs, FirestoreSerializer } from './utils'
+import {
+  createSnapshot,
+  extractRefs,
+  firestoreDefaultConverter,
+  FirestoreSerializer,
+} from './utils'
 import { walkGet, callOnceWithArg, OperationsType } from '../shared'
 import { ref, Ref, unref } from 'vue-demi'
 import type {
@@ -7,21 +12,29 @@ import type {
   DocumentData,
   DocumentReference,
   DocumentSnapshot,
+  FirestoreDataConverter,
   Query,
 } from 'firebase/firestore'
 import { onSnapshot } from 'firebase/firestore'
 
 export interface FirestoreOptions {
   maxRefDepth?: number
   reset?: boolean | (() => any)
+  /**
+   * @deprecated use `converter` instead
+   */
   serialize?: FirestoreSerializer
+
+  converter?: FirestoreDataConverter<unknown>
+
   wait?: boolean
 }
 
 const DEFAULT_OPTIONS: Required<FirestoreOptions> = {
   maxRefDepth: 2,
   reset: true,
   serialize: createSnapshot,
+  converter: firestoreDefaultConverter,
   wait: false,
 }
 export { DEFAULT_OPTIONS as firestoreOptions }

src/firestore/utils.ts

@@ -1,10 +1,12 @@
-import type {
+import {
   Query,
   DocumentReference,
   CollectionReference,
   DocumentSnapshot,
   DocumentData,
   GeoPoint,
+  doc,
+  FirestoreDataConverter,
 } from 'firebase/firestore'
 import { isTimestamp, isObject, isDocumentRef, TODO } from '../shared'
 
@@ -21,6 +23,28 @@ export function createSnapshot<T = DocumentData>(
   return Object.defineProperty(doc.data() || {}, 'id', { value: doc.id })
 }
 
+export const firestoreDefaultConverter: FirestoreDataConverter<unknown> = {
+  toFirestore(data) {
+    // this is okay because we declare other properties as non-enumerable
+    return data as DocumentData
+  },
+  fromFirestore(snapshot, options) {
+    return snapshot.exists()
+      ? Object.defineProperties(snapshot.data(options)!, {
+          id: {
+            // TODO: can the `id` change? If so this should be a get
+            value: () => snapshot.id,
+          },
+          // TODO: check if worth adding or should be through an option
+          // $meta: {
+          //   value: snapshot.metadata,
+          // },
+          // $ref: { get: () => snapshot.ref },
+        })
+      : null
+  },
+}
+
 export type FirestoreSerializer = typeof createSnapshot
 
 export function extractRefs(
@@ -66,6 +90,7 @@ export function extractRefs(
       if (
         // primitives
         ref == null ||
+        // TODO: check and remove
         // Firestore < 4.13
         ref instanceof Date ||
         isTimestamp(ref) ||

src/shared.ts

@@ -2,6 +2,8 @@ import type {
   CollectionReference,
   DocumentData,
   DocumentReference,
+  DocumentSnapshot,
+  QuerySnapshot,
 } from 'firebase/firestore'
 import type { Ref } from 'vue-demi'
 
@@ -18,7 +20,8 @@ export interface OperationsType {
   remove<T extends unknown = unknown>(array: T[], index: number): T[]
 }
 
-export type ResetOption = boolean | (() => any)
+// TODO: probably boolean | void or just boolean
+export type ResetOption = boolean | (() => TODO)
 
 export type TODO = any
 /**
@@ -92,8 +95,6 @@ export function isCollectionRef(o: any): o is CollectionReference {
   return isObject(o) && o.type === 'collection'
 }
 
-type ReferenceType = 'collection' | 'document' | 'query'
-
 /**
  * Wraps a function so it gets called only once
  * @param fn Function to be called once

src/vuefire/firestore.ts

@@ -10,6 +10,7 @@ import type {
   DocumentReference,
   Query,
   FirestoreError,
+  DocumentData,
 } from 'firebase/firestore'
 import {
   getCurrentInstance,
@@ -22,8 +23,6 @@ import {
 import { isDocumentRef, _RefWithState } from '../shared'
 import { firestoreUnbinds } from './optionsApi'
 
-export interface _RefFirestore<T> extends _RefWithState<T, FirestoreError> {}
-
 export const ops: OperationsType = {
   set: (target, key, value) => walkSet(target, key, value),
   add: (array, index, data) => array.splice(index, 0, data),
@@ -162,7 +161,7 @@ export function useCollection<
 >(
   collectionRef: R,
   options?: UseCollectionOptions
-): Ref<_InferReferenceType<R>[]>
+): _RefFirestore<_InferReferenceType<R>[]>
 
 /**
  * Creates a reactive collection (usually an array) of documents from a collection ref or a query from Firestore.
@@ -175,13 +174,15 @@ export function useCollection<
 export function useCollection<T>(
   collectionRef: CollectionReference | Query,
   options?: UseCollectionOptions
-): Ref<T[]>
+): _RefFirestore<VueFireQueryData<T>>
 
 export function useCollection<T>(
   collectionRef: CollectionReference<unknown> | Query<unknown>,
   options?: UseCollectionOptions
-): Ref<_InferReferenceType<T>[]> | Ref<T[]> {
-  return _useFirestoreRef(collectionRef, options) as _RefFirestore<T[]>
+): _RefFirestore<VueFireQueryData<T>> {
+  return _useFirestoreRef(collectionRef, options) as _RefFirestore<
+    VueFireQueryData<T>
+  >
 }
 
 export interface UseDocumentOptions {}
@@ -198,7 +199,7 @@ export function useDocument<
 >(
   documentRef: R,
   options?: UseDocumentOptions
-): _RefFirestore<_InferReferenceType<R> | null>
+): _RefFirestore<_InferReferenceType<R>> // this one can't be null or should be specified in the converter
 
 /**
  * Creates a reactive collection (usually an array) of documents from a collection ref or a query from Firestore.
@@ -211,7 +212,7 @@ export function useDocument<
 export function useDocument<T>(
   documentRef: DocumentReference,
   options?: UseDocumentOptions
-): _RefFirestore<T | null>
+): _RefFirestore<VueFireDocumentData<T>>
 
 export function useDocument<T>(
   documentRef: DocumentReference<unknown>,
@@ -248,3 +249,21 @@ export type _InferReferenceType<R> = R extends
   | DocumentReference<infer T>
   ? T
   : R
+
+/**
+ * Type used by default by the `firestoreDefaultConverter`.
+ */
+export type VueFireDocumentData<T = DocumentData> =
+  | null
+  | (T & {
+      /**
+       * id of the document
+       */
+      readonly id: string
+    })
+
+export type VueFireQueryData<T = DocumentData> = Array<
+  Exclude<VueFireDocumentData<T>, null>
+>
+
+export interface _RefFirestore<T> extends _RefWithState<T, FirestoreError> {}

tests/firestore/collection.spec.ts

@@ -46,21 +46,36 @@ describe('Firestore collections', () => {
   })
 
   tds(() => {
+    interface TodoI {
+      text: string
+      finished: boolean
+    }
+
     const db = firestore
     const collection = originalCollection
     expectType<Ref<DocumentData[]>>(useCollection(collection(db, 'todos')))
-    // @ts-expect-error
+    // @ts-expect-error: document data by default
     expectType<Ref<number[]>>(useCollection(collection(db, 'todos')))
 
-    expectType<Ref<number[]>>(useCollection<number>(collection(db, 'todos')))
-    // @ts-expect-error
-    expectType<Ref<string[]>>(useCollection<number>(collection(db, 'todos')))
+    expectType<Ref<TodoI[]>>(useCollection<TodoI>(collection(db, 'todos')))
+    expectType<Ref<TodoI[]>>(useCollection<TodoI>(collection(db, 'todos')).data)
+    expectType<string>(
+      useCollection<TodoI>(collection(db, 'todos')).value.at(0)!.id
+    )
+    expectType<string>(
+      useCollection<TodoI>(collection(db, 'todos')).data.value.at(0)!.id
+    )
+    // @ts-expect-error: wrong type
+    expectType<Ref<string[]>>(useCollection<TodoI>(collection(db, 'todos')))
 
     const refWithConverter = collection(db, 'todos').withConverter<number>({
       toFirestore: (data) => ({ n: data }),
       fromFirestore: (snap, options) => snap.data(options).n as number,
     })
     expectType<Ref<number[]>>(useCollection(refWithConverter))
+    expectType<Ref<number[]>>(useCollection(refWithConverter).data)
+    // @ts-expect-error: no id with converter
+    expectType<Ref<number[]>>(useCollection(refWithConverter).data.value.id)
     // @ts-expect-error
     expectType<Ref<string[]>>(useCollection(refWithConverter))
   })

tests/firestore/document.spec.ts

@@ -48,16 +48,22 @@ describe('Firestore documents', () => {
     expectType<Ref<number | null>>(useDocument(itemRef))
 
     expectType<Ref<number | null>>(useDocument<number>(itemRef))
+    expectType<Ref<number | null>>(useDocument<number>(itemRef).data)
     // @ts-expect-error
     expectType<Ref<string | null>>(useDocument<number>(itemRef))
 
     const refWithConverter = itemRef.withConverter<number>({
       toFirestore: (data) => ({ n: data }),
       fromFirestore: (snap, options) => snap.data(options).n as number,
     })
-    expectType<Ref<number | null>>(useDocument(refWithConverter))
-    // @ts-expect-error
-    expectType<Ref<string | null>>(useDocument(refWithConverter))
+    expectType<Ref<number>>(useDocument(refWithConverter))
+    expectType<Ref<number>>(useDocument(refWithConverter).data)
+    // should not be null
+    useDocument(refWithConverter).value.toFixed(14)
+    // @ts-expect-error: string is not assignable to number
+    expectType<Ref<string>>(useDocument(refWithConverter))
+    // @ts-expect-error: no id when a custom converter is used
+    useDocument(refWithConverter).value.id
 
     // destructuring
     expectType<Ref<DocumentData | null>>(useDocument(itemRef).data)