fix(firestore): correctly set pending when hydrating and during racing conditions

Author: posva

Diff for: src/database/bind.ts

@@ -13,7 +13,7 @@ import {
   _MaybeRef,
   _ResolveRejectFn,
 } from '../shared'
-import { ref, Ref, unref } from 'vue-demi'
+import { Ref, unref } from 'vue-demi'
 import {
   onValue,
   onChildAdded,

Diff for: src/database/useDatabaseRef.ts

@@ -41,7 +41,7 @@ export function _useDatabaseRef(
   reference: _MaybeRef<_Nullable<DatabaseReference | Query>>,
   localOptions: UseDatabaseRefOptions = {}
 ): _RefDatabase<unknown> {
-  let unbind!: UnbindWithReset
+  let unbind: UnbindWithReset = noop
   const options = Object.assign({}, globalDatabaseOptions, localOptions)
   const initialSourceValue = unref(reference)
   const data = options.target || ref<unknown | null>()
@@ -84,6 +84,7 @@ export function _useDatabaseRef(
     const referenceValue = unref(reference)
 
     const newPromise = new Promise<unknown | null>((resolve, reject) => {
+      unbind(options.reset)
       if (!referenceValue) {
         unbind = noop
         // resolve to avoid an ever pending promise

Diff for: src/firestore/useFirestoreRef.ts

@@ -81,15 +81,17 @@ export function _useFirestoreRef(
   }
 
   // set the initial value from SSR even if the ref comes from outside
-  // TODO: allow passing firebase app name
   data.value = getInitialValue(
     initialSourceValue,
     options.ssrKey,
     data.value,
     useFirebaseApp()
   )
 
-  const pending = ref(true)
+  // if no initial value is found (ssr), we should set pending to true
+  let shouldStartAsPending = data.value === undefined // no initial value
+
+  const pending = ref(false)
   const error = ref<FirestoreError>()
   // force the type since its value is set right after and undefined isn't possible
   const promise = shallowRef() as ShallowRef<Promise<unknown | null>>
@@ -99,7 +101,7 @@ export function _useFirestoreRef(
   function bindFirestoreRef() {
     let docRefValue = unref(docOrCollectionRef)
 
-    const p = new Promise<unknown | null>((resolve, reject) => {
+    const newPromise = new Promise<unknown | null>((resolve, reject) => {
       // stop the previous subscription
       unbind(options.reset)
       // skip if the ref is null or undefined
@@ -110,6 +112,11 @@ export function _useFirestoreRef(
         return resolve(null)
       }
 
+      pending.value = shouldStartAsPending
+      // the very first time we bind, if we hydrated the value, we don't set loading to true
+      // this way we ensure, all subsequent calls to bindDatabaseRef will set pending to true
+      shouldStartAsPending = true
+
       if (!docRefValue.converter) {
         docRefValue = docRefValue.withConverter(
           // @ts-expect-error: seems like a ts error
@@ -128,25 +135,30 @@ export function _useFirestoreRef(
         options
       )
     })
-
-    promise.value = p
-
-    p.catch((reason: FirestoreError) => {
-      error.value = reason
-    }).finally(() => {
-      pending.value = false
-    })
+      .catch((reason) => {
+        if (promise.value === newPromise) {
+          error.value = reason
+        }
+        return Promise.reject(reason) // propagate the error
+      })
+      .finally(() => {
+        // ensure the current promise is still valid
+        if (promise.value === newPromise) {
+          pending.value = false
+        }
+      })
+
+    // we set the promise here to ensure that pending is set right after if the user awaits the promise
+    promise.value = newPromise
   }
 
   let stopWatcher = noop
   if (isRef(docOrCollectionRef)) {
-    stopWatcher = watch(docOrCollectionRef, bindFirestoreRef, {
-      immediate: true,
-    })
-  } else {
-    bindFirestoreRef()
+    stopWatcher = watch(docOrCollectionRef, bindFirestoreRef)
   }
 
+  bindFirestoreRef()
+
   // only add the first promise to the pending ones
   if (initialSourceValue) {
     removePendingPromise = addPendingPromise(

Diff for: tests/database/ssr.spec.ts

@@ -8,12 +8,15 @@ import { useDatabaseObject } from '../../src'
 import { createSSRApp } from 'vue'
 import { renderToString, ssrInterpolate } from '@vue/server-renderer'
 import { clearPendingPromises } from '../../src/ssr/plugin'
+import { _initialStatesMap } from '../../src/ssr/initialState'
 
 describe('Database SSR', async () => {
   const { databaseRef, set } = setupDatabaseRefs()
 
   beforeEach(() => {
     clearPendingPromises(firebaseApp)
+    // delete any ssr state
+    _initialStatesMap.delete(firebaseApp)
   })
 
   function createMyApp<T>(

Diff for: tests/firestore/document.spec.ts

@@ -1,13 +1,19 @@
 import { mount } from '@vue/test-utils'
-import { describe, expect, it } from 'vitest'
+import { beforeEach, describe, expect, it } from 'vitest'
 import {
   addDoc,
   doc as originalDoc,
   DocumentData,
   DocumentReference,
   FirestoreError,
 } from 'firebase/firestore'
-import { expectType, setupFirestoreRefs, tds, firestore } from '../utils'
+import {
+  expectType,
+  setupFirestoreRefs,
+  tds,
+  firestore,
+  firebaseApp,
+} from '../utils'
 import { nextTick, ref, shallowRef, unref, type Ref } from 'vue'
 import { isPOJO, _MaybeRef, _Nullable } from '../../src/shared'
 import {
@@ -17,6 +23,10 @@ import {
   _RefFirestore,
 } from '../../src'
 import { mockWarn } from '../vitest-mock-warn'
+import {
+  useSSRInitialState,
+  _initialStatesMap,
+} from '../../src/ssr/initialState'
 
 describe(
   'Firestore documents',
@@ -57,6 +67,11 @@ describe(
       }
     }
 
+    beforeEach(() => {
+      // delete any ssr state
+      _initialStatesMap.delete(firebaseApp)
+    })
+
     it('binds a document', async () => {
       const { wrapper, itemRef, data } = factory()
 
@@ -118,6 +133,45 @@ describe(
       expect(data.value!.id).toBe(itemRef.id)
     })
 
+    it('sets pending while loading', async () => {
+      const itemRef = shallowRef(doc('a'))
+      const { pending, promise } = factory({ ref: itemRef })
+
+      expect(pending.value).toBe(true)
+      await promise.value
+      expect(pending.value).toBe(false)
+
+      // set the target to a new ref so it can be loaded again
+      itemRef.value = doc('b')
+
+      await nextTick() // for the watcher to trigger
+      expect(pending.value).toBe(true)
+      await promise.value
+      expect(pending.value).toBe(false)
+    })
+
+    it('sets pending to false if there is an initial value (ssr)', async () => {
+      const itemRef = shallowRef(doc())
+      useSSRInitialState({ f: { a: 1 }, r: {}, s: {}, u: {} }, firebaseApp)
+      const { pending, promise } = factory({
+        ref: itemRef,
+        options: { ssrKey: 'a' },
+      })
+
+      expect(pending.value).toBe(false)
+      await promise.value
+      expect(pending.value).toBe(false)
+    })
+
+    it('skips setting pending if the object is an empty ref', async () => {
+      const itemRef = shallowRef()
+      const { pending, promise } = factory({ ref: itemRef })
+
+      expect(pending.value).toBe(false)
+      await promise.value
+      expect(pending.value).toBe(false)
+    })
+
     it('manually unbinds a document', async () => {
       const { itemRef, data, stop: unbind } = factory()
 

Diff for: tests/firestore/ssr.spec.ts

@@ -8,13 +8,16 @@ import { useDocument } from '../../src'
 import { createSSRApp } from 'vue'
 import { renderToString, ssrInterpolate } from '@vue/server-renderer'
 import { clearPendingPromises } from '../../src/ssr/plugin'
+import { _initialStatesMap } from '../../src/ssr/initialState'
 
 describe('Firestore SSR', async () => {
   const { collection, query, addDoc, setDoc, updateDoc, deleteDoc, doc } =
     setupFirestoreRefs()
 
   beforeEach(() => {
     clearPendingPromises(firebaseApp)
+    // delete any ssr state
+    _initialStatesMap.delete(firebaseApp)
   })
 
   function createMyApp<T>(