feat(firestore): allow custom converter

Close #608

BREAKING CHANGE: `options.serialize()` is replaced with `converter`. It
effectively has the same effect as calling `doc().withConverter()` or
`collection().withConverter()` but it allows to have a global converter
that is automatically applied to all snapshots. This custom converter
adds a non-enumerable `id` property for documents like the previous
`serialize` options. **If you were not using this option**, you don't
need to change anything.

src/firestore/index.ts

@@ -20,10 +20,6 @@ import { onSnapshot } from 'firebase/firestore'
 export interface FirestoreOptions {
   maxRefDepth?: number
   reset?: boolean | (() => any)
-  /**
-   * @deprecated use `converter` instead
-   */
-  serialize?: FirestoreSerializer
 
   converter?: FirestoreDataConverter<unknown>
 
@@ -33,7 +29,6 @@ export interface FirestoreOptions {
 const DEFAULT_OPTIONS: Required<FirestoreOptions> = {
   maxRefDepth: 2,
   reset: true,
-  serialize: createSnapshot,
   converter: firestoreDefaultConverter,
   wait: false,
 }
@@ -65,7 +60,9 @@ function updateDataFromDocumentSnapshot<T>(
   resolve: CommonBindOptionsParameter['resolve']
 ) {
   const [data, refs] = extractRefs(
-    options.serialize(snapshot),
+    // @ts-expect-error: FIXME: use better types
+    // Pass snapshot options
+    snapshot.data(),
     walkGet(target, path),
     subs
   )
@@ -200,7 +197,7 @@ interface BindCollectionParameter extends CommonBindOptionsParameter {
   collection: CollectionReference | Query
 }
 
-export function bindCollection<T>(
+export function bindCollection<T = unknown>(
   target: BindCollectionParameter['target'],
   collection: CollectionReference<T> | Query<T>,
   ops: BindCollectionParameter['ops'],
@@ -209,11 +206,15 @@ export function bindCollection<T>(
   extraOptions: FirestoreOptions = DEFAULT_OPTIONS
 ) {
   const options = Object.assign({}, DEFAULT_OPTIONS, extraOptions) // fill default values
-  // a custom converter means we don't need a serializer
-  if (collection.converter) {
-    // @ts-expect-error: FIXME: remove this serialize option
-    options.serialize = (v) => v.data()
+
+  if (!collection.converter) {
+    // @ts-expect-error: seems like a ts error
+    collection = collection.withConverter(
+      // @ts-expect-error: seems like a ts error
+      options.converter as FirestoreDataConverter<T>
+    )
   }
+
   const key = 'value'
   if (!options.wait) ops.set(target, key, [])
   let arrayRef = ref(options.wait ? [] : target[key])
@@ -228,7 +229,13 @@ export function bindCollection<T>(
     added: ({ newIndex, doc }: DocumentChange<T>) => {
       arraySubs.splice(newIndex, 0, Object.create(null))
       const subs = arraySubs[newIndex]
-      const [data, refs] = extractRefs(options.serialize(doc), undefined, subs)
+      // FIXME: wrong cast, needs better types
+      // TODO: pass SnapshotOptions
+      const [data, refs] = extractRefs(
+        doc.data() as DocumentData,
+        undefined,
+        subs
+      )
       ops.add(unref(arrayRef), newIndex, data)
       subscribeToRefs(
         options,
@@ -245,7 +252,9 @@ export function bindCollection<T>(
       const array = unref(arrayRef)
       const subs = arraySubs[oldIndex]
       const oldData = array[oldIndex]
-      const [data, refs] = extractRefs(options.serialize(doc), oldData, subs)
+      // @ts-expect-error: FIXME: Better types
+      // TODO: pass SnapshotOptions
+      const [data, refs] = extractRefs(doc.data(), oldData, subs)
       // only move things around after extracting refs
       // only move things around after extracting refs
       arraySubs.splice(newIndex, 0, subs)

src/firestore/utils.ts

@@ -9,6 +9,7 @@ import {
   FirestoreDataConverter,
 } from 'firebase/firestore'
 import { isTimestamp, isObject, isDocumentRef, TODO } from '../shared'
+import { VueFireDocumentData } from '../vuefire/firestore'
 
 export type FirestoreReference = Query | DocumentReference | CollectionReference
 
@@ -23,27 +24,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 const firestoreDefaultConverter: FirestoreDataConverter<VueFireDocumentData> =
+  {
+    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 },
+          }) as VueFireDocumentData)
+        : null
+    },
+  }
 
 export type FirestoreSerializer = typeof createSnapshot
 

src/vuefire/index.ts

@@ -12,7 +12,11 @@ export {
   useDocument,
 } from './firestore'
 
-export type { UseCollectionOptions } from './firestore'
+export type {
+  UseCollectionOptions,
+  VueFireDocumentData,
+  VueFireQueryData,
+} from './firestore'
 
 export { firestorePlugin } from './optionsApi'
 export type {

src/vuefire/optionsApi.ts

@@ -82,15 +82,15 @@ export const firestoreUnbinds = new WeakMap<
 export interface PluginOptions {
   bindName?: string
   unbindName?: string
-  serialize?: FirestoreOptions['serialize']
+  converter?: FirestoreOptions['converter']
   reset?: FirestoreOptions['reset']
   wait?: FirestoreOptions['wait']
 }
 
 const defaultOptions: Readonly<Required<PluginOptions>> = {
   bindName: '$bind',
   unbindName: '$unbind',
-  serialize: firestoreOptions.serialize,
+  converter: firestoreOptions.converter,
   reset: firestoreOptions.reset,
   wait: firestoreOptions.wait,
 }

tests/firestore/options.spec.ts

@@ -35,8 +35,14 @@ describe('Firestore: Options API', () => {
 
   it('calls custom serialize function with collection', async () => {
     const pluginOptions: PluginOptions = {
-      // @ts-expect-error: FIXME:
-      serialize: vi.fn(() => ({ foo: 'bar' })),
+      converter: {
+        fromFirestore: vi.fn((snapshot, options?) => ({
+          foo: 'bar',
+        })),
+        toFirestore(data: DocumentData) {
+          return data
+        },
+      },
     }
     const wrapper = mount(
       {
@@ -55,16 +61,24 @@ describe('Firestore: Options API', () => {
 
     await wrapper.vm.$bind('items', itemsRef)
 
-    expect(pluginOptions.serialize).toHaveBeenCalledTimes(1)
-    expect(pluginOptions.serialize).toHaveBeenCalledWith(
-      expect.objectContaining({ data: expect.any(Function) })
+    expect(pluginOptions.converter?.fromFirestore).toHaveBeenCalledTimes(1)
+    expect(pluginOptions.converter?.fromFirestore).toHaveBeenCalledWith(
+      expect.objectContaining({ data: expect.any(Function) }),
+      expect.anything()
     )
     expect(wrapper.vm.items).toEqual([{ foo: 'bar' }])
   })
 
   it('can be overridden by local option', async () => {
-    const pluginOptions = {
-      serialize: vi.fn(() => ({ foo: 'bar' })),
+    const pluginOptions: PluginOptions = {
+      converter: {
+        fromFirestore: vi.fn((snapshot, options?) => ({
+          foo: 'bar',
+        })),
+        toFirestore(data: DocumentData) {
+          return data
+        },
+      },
     }
     const wrapper = mount(
       {
@@ -83,13 +97,20 @@ describe('Firestore: Options API', () => {
 
     const spy = vi.fn(() => ({ bar: 'bar' }))
 
-    // @ts-expect-error: FIXME:
-    await wrapper.vm.$bind('items', itemsRef, { serialize: spy })
+    await wrapper.vm.$bind('items', itemsRef, {
+      converter: {
+        fromFirestore: spy,
+        toFirestore(data: DocumentData) {
+          return data
+        },
+      },
+    })
 
-    expect(pluginOptions.serialize).not.toHaveBeenCalled()
+    expect(pluginOptions.converter?.fromFirestore).not.toHaveBeenCalled()
     expect(spy).toHaveBeenCalledTimes(1)
     expect(spy).toHaveBeenCalledWith(
-      expect.objectContaining({ data: expect.any(Function) })
+      expect.objectContaining({ data: expect.any(Function) }),
+      expect.anything()
     )
     expect(wrapper.vm.items).toEqual([{ bar: 'bar' }])
   })