fix(firestore): set pending on queries

Fix #1317

Author: posva

src/firestore/useFirestoreRef.ts

@@ -31,7 +31,6 @@ import {
   OperationsType,
   walkSet,
   _RefWithState,
-  isCollectionRef,
 } from '../shared'
 import { getInitialValue } from '../ssr/initialState'
 import { addPendingPromise } from '../ssr/plugin'
@@ -50,6 +49,8 @@ export interface _UseFirestoreRefOptions extends FirestoreRefOptions {
   converter?: FirestoreDataConverter<unknown>
 }
 
+const NO_INITIAL_VALUE = Symbol()
+
 /**
  * Internal version of `useDocument()` and `useCollection()`.
  *
@@ -87,18 +88,14 @@ export function _useFirestoreRef(
   const initialValue = getInitialValue(
     initialSourceValue,
     options.ssrKey,
-    data.value,
+    NO_INITIAL_VALUE,
     useFirebaseApp()
   )
-  data.value = initialValue
-  const hasInitialValue =
-    // TODO: we need a stricter check for collections and queries and the initial target is passed as a ref([]) but
-    // maybe that [] should be set here instead. It's also worth taking into account that a custom ref can be passed as
-    // target as it should probably be initially empty but maybe this is too much to ask.
-    // TODO: add and test || isFirestoreQuery()
-    isCollectionRef(initialSourceValue)
-      ? ((initialValue || []) as unknown[]).length > 0
-      : initialValue !== undefined
+
+  const hasInitialValue = initialValue !== NO_INITIAL_VALUE
+  if (hasInitialValue) {
+    data.value = initialValue
+  }
 
   // if no initial value is found (ssr), we should set pending to true
   let shouldStartAsPending = !hasInitialValue

src/shared.ts

@@ -121,9 +121,12 @@ export function isFirestoreDataReference<T = unknown>(
   return isDocumentRef(source) || isCollectionRef(source)
 }
 
-export function isFirestoreQuery(
+export function isFirestoreQuery<
+  AppModelType = DocumentData,
+  DbModelType extends DocumentData = DocumentData
+>(
   source: unknown
-): source is FirestoreQuery<unknown> & { path: undefined } {
+): source is FirestoreQuery<AppModelType, DbModelType> & { path: undefined } {
   // makes some types so much easier
   return isObject(source) && source.type === 'query'
 }

tests/firestore/collection.spec.ts

@@ -71,14 +71,17 @@ describe(
       }
     }
 
-    function factoryQuery<T = DocumentData>({
+    function factoryQuery<
+      AppModelType = DocumentData,
+      DbModelType extends DocumentData = DocumentData
+    >({
       options,
       ref,
     }: {
       options?: UseCollectionOptions
-      ref?: _MaybeRef<_Nullable<CollectionReference<T> | Query<T>>>
+      ref?: _MaybeRef<_Nullable<Query<AppModelType, DbModelType>>>
     } = {}) {
-      let data!: _RefFirestore<VueFirestoreQueryData<T>>
+      let data!: _RefFirestore<VueFirestoreQueryData<AppModelType>>
 
       const wrapper = mount(
         defineComponent({
@@ -88,7 +91,10 @@ describe(
             data = useCollection(
               // split for ts
               ref,
-              options
+              {
+                ssrKey: Math.random().toString(36),
+                ...options,
+              }
             )
             const { data: list, pending, error, promise, stop } = data
             return { list, pending, error, promise, stop }
@@ -100,6 +106,7 @@ describe(
         wrapper,
         // non enumerable properties cannot be spread
         data: data.data,
+        listQuery: unref(ref)!,
         pending: data.pending,
         error: data.error,
         promise: data.promise,
@@ -175,6 +182,19 @@ describe(
       expect(pending.value).toBe(false)
     })
 
+    it('sets pending while loading with a query', async () => {
+      const listRef = collection<{ name: string }>()
+      const { wrapper, listQuery, pending, promise } = factoryQuery<{
+        name: string
+      }>({
+        ref: query(listRef, where('name', '==', 'a')),
+      })
+
+      expect(pending.value).toBe(true)
+      await promise.value
+      expect(pending.value).toBe(false)
+    })
+
     it('fetches once', async () => {
       const listRef = collection<{ name: string }>()
       await addDoc(listRef, { name: 'a' })
@@ -497,5 +517,5 @@ describe(
       expectType<Ref<string[]>>(useCollection(refWithConverter))
     })
   },
-  { retry: 3 }
+  { retry: process.env.CI ? 3 : 1 }
 )