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

Author: posva

src/database/useDatabaseRef.ts

@@ -63,31 +63,38 @@ export function _useDatabaseRef(
   }
 
   // set the initial value from SSR even if the ref comes from outside
-  // TODO: firebase app name
   data.value = getInitialValue(
     initialSourceValue,
     options.ssrKey,
     data.value,
     useFirebaseApp()
   )
 
+  // if no initial value is found (ssr), we should set pending to true
+  let shouldStartAsPending = data.value === undefined // no initial value
+
   const error = ref<Error>()
-  const pending = ref(true)
+  const pending = ref(false)
   // force the type since its value is set right after and undefined isn't possible
   const promise = shallowRef() as ShallowRef<Promise<unknown | null>>
   const hasCurrentScope = getCurrentScope()
   let removePendingPromise = noop
 
   function bindDatabaseRef() {
-    let referenceValue = unref(reference)
+    const referenceValue = unref(reference)
 
-    const p = new Promise<unknown | null>((resolve, reject) => {
+    const newPromise = new Promise<unknown | null>((resolve, reject) => {
       if (!referenceValue) {
         unbind = noop
         // resolve to avoid an ever pending promise
         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 (Array.isArray(data.value)) {
         unbind = bindAsArray(
           data as Ref<any>,
@@ -100,22 +107,28 @@ export function _useDatabaseRef(
         unbind = bindAsObject(data, referenceValue, resolve, reject, options)
       }
     })
-
-    promise.value = p
-
-    p.catch((reason) => {
-      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(reference)) {
-    stopWatcher = watch(reference, bindDatabaseRef, { immediate: true })
-  } else {
-    bindDatabaseRef()
+    stopWatcher = watch(reference, bindDatabaseRef)
   }
+  bindDatabaseRef()
 
   // only add the first promise to the pending ones
   if (initialSourceValue) {
@@ -137,6 +150,8 @@ export function _useDatabaseRef(
     unbind(reset)
   }
 
+  // TODO: warn if the data has already any property set (use a symbol to check in dev)
+
   return Object.defineProperties(data as _RefDatabase<unknown>, {
     // allow destructuring without interfering with the ref itself
     data: { get: () => data },

src/ssr/initialState.ts

@@ -31,7 +31,7 @@ export interface SSRStore {
 }
 
 // @internal
-const initialStatesMap = new WeakMap<FirebaseApp, SSRStore>()
+export const _initialStatesMap = new WeakMap<FirebaseApp, SSRStore>()
 
 /**
  * Allows getting the initial state set during SSR on the client.
@@ -45,14 +45,14 @@ export function useSSRInitialState(
   firebaseApp: FirebaseApp
 ): SSRStore {
   // get initial state based on the current firebase app
-  if (!initialStatesMap.has(firebaseApp)) {
-    initialStatesMap.set(
+  if (!_initialStatesMap.has(firebaseApp)) {
+    _initialStatesMap.set(
       firebaseApp,
       initialState || { f: {}, r: {}, s: {}, u: {} }
     )
   }
 
-  return initialStatesMap.get(firebaseApp)!
+  return _initialStatesMap.get(firebaseApp)!
 }
 
 export function getInitialValue(

tests/database/objects.spec.ts

@@ -1,16 +1,24 @@
 import { mount } from '@vue/test-utils'
-import { describe, expect, it } from 'vitest'
+import { beforeEach, describe, expect, it } from 'vitest'
 import {
   UseDatabaseRefOptions,
   useDatabaseObject,
   VueDatabaseDocumentData,
   _RefDatabase,
+  useSSRInitialState,
 } from '../../src'
-import { expectType, tds, setupDatabaseRefs, database } from '../utils'
+import {
+  expectType,
+  tds,
+  setupDatabaseRefs,
+  database,
+  firebaseApp,
+} from '../utils'
 import { computed, nextTick, ref, shallowRef, unref, type Ref } from 'vue'
 import { DatabaseReference, get, ref as _databaseRef } from 'firebase/database'
 import { _MaybeRef, _Nullable } from '../../src/shared'
 import { mockWarn } from '../vitest-mock-warn'
+import { _initialStatesMap } from '../../src/ssr/initialState'
 
 describe('Database objects', () => {
   const { databaseRef, set, update, remove } = setupDatabaseRefs()
@@ -46,6 +54,11 @@ describe('Database objects', () => {
     }
   }
 
+  beforeEach(() => {
+    // delete any ssr state
+    _initialStatesMap.delete(firebaseApp)
+  })
+
   it('binds an object', async () => {
     const { wrapper, itemRef } = factory()
 
@@ -74,6 +87,45 @@ describe('Database objects', () => {
     expect(wrapper.vm.item).toBe(null)
   })
 
+  it('sets pending while loading', async () => {
+    const itemRef = shallowRef(databaseRef('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 = databaseRef('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(databaseRef())
+    useSSRInitialState({ r: { a: 1 }, f: {}, 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('retrieves an object with $value for primitives', async () => {
     const itemRef = databaseRef()
     await set(itemRef, 24)