chore: some todos

Author: posva

src/app/index.ts

@@ -12,7 +12,6 @@ export const _FirebaseAppInjectionKey: InjectionKey<FirebaseApp> =
  * @returns the firebase app
  */
 export function useFirebaseApp(name?: string): FirebaseApp {
-  // TODO: warn no current scope
   return (
     (getCurrentInstance() &&
       inject(

src/auth/index.ts

@@ -26,7 +26,7 @@ export function VueFireAuth(_app?: never) {
   //                        ^
   // app: never to prevent the user from just passing `VueFireAuth` without calling the function
 
-  // TODO: refactor to share across modules
+  // TODO: Hopefully we should be able to remove this with the next Vue release
   if (process.env.NODE_ENV !== 'production') {
     if (_app != null) {
       console.warn(`Did you forget to call the VueFireAuth function? It should look like

src/auth/user.ts

@@ -10,7 +10,7 @@ export const AuthUserInjectSymbol: InjectionKey<Ref<User | null | undefined>> =
  * authenticated or when the user logs out. The ref is undefined when the user is not yet loaded.
  */
 export function useCurrentUser() {
-  // TODO: warn no current instance
+  // TODO: warn no current instance in DEV
   return inject(AuthUserInjectSymbol)!
 }
 

src/database/index.ts

@@ -75,7 +75,7 @@ export function _useDatabaseRef(
     const p = new Promise<unknown | null>((resolve, reject) => {
       if (!referenceValue) {
         _unbind = noop
-        // TODO: maybe we shouldn't resolve this at all?
+        // resolve to avoid an ever pending promise
         return resolve(null)
       }
 
@@ -147,7 +147,8 @@ export function _useDatabaseRef(
     _unbind(reset)
   }
 
-  return Object.defineProperties(data, {
+  return Object.defineProperties(data as _RefDatabase<unknown>, {
+    // allow destructuring without interfering with the ref itself
     data: { get: () => data },
     error: { get: () => error },
     pending: { get: () => error },
@@ -166,9 +167,6 @@ export function internalUnbind(
     unbinds[key](reset)
     delete unbinds[key]
   }
-  // TODO: move to $firestoreUnbind
-  // delete vm._firebaseSources[key]
-  // delete vm._firebaseUnbinds[key]
 }
 
 export type UseListOptions = UseDatabaseRefOptions

src/database/optionsApi.ts

@@ -73,6 +73,8 @@ export function databasePlugin(
     internalUnbind(key, databaseUnbinds.get(this), reset)
     // @ts-expect-error: readonly for the users
     delete this.$firebaseRefs[key]
+    // delete this._firebaseSources[key]
+    // delete this._firebaseUnbinds[key]
   }
 
   GlobalTarget[bindName] = function databaseBind(

src/database/subscribe.ts

@@ -121,6 +121,7 @@ export function bindAsArray(
       ops.add(array, index, options.serialize(snapshot))
     }
     // TODO: cancelcallback
+    // reject,
   )
 
   const removeChildRemovedListener = onChildRemoved(
@@ -158,13 +159,14 @@ export function bindAsArray(
     // TODO: cancelcallback
   )
 
-  const removeValueListener = onValue(
+  // in case the removeValueListener() is called before onValue returns
+  let removeValueListener = noop
+  removeValueListener = onValue(
     collection,
     (data) => {
       const array = unref(arrayRef)
       if (options.wait) ops.set(target, key, array)
       resolve(data)
-      // FIXME: this can be called before it's initialized
       removeValueListener()
     },
     reject

src/firestore/index.ts

@@ -70,7 +70,7 @@ export function _useFirestoreRef(
   const data = options.target || ref<unknown | null>()
   // set the initial value from SSR even if the ref comes from outside
   data.value = getInitialValue(initialSourceValue, options.ssrKey, data.value)
-  // TODO: allow passing pending and error refs as option for when this is called using the options api
+
   const pending = ref(true)
   const error = ref<FirestoreError>()
   // force the type since its value is set right after and undefined isn't possible
@@ -141,12 +141,11 @@ export function _useFirestoreRef(
   // create sync, you can read only once the whole thing so maybe we
   // should take an option like once: true to not setting up any listener
 
-  // TODO: warn else
   if (hasCurrentScope) {
     onScopeDispose(unbind)
     if (getCurrentInstance()) {
       // wait for the promise during SSR
-      // TODO: configurable
+      // TODO: configurable ssrKey: false to disable this
       onServerPrefetch(() => promise.value)
     }
   }
@@ -159,26 +158,13 @@ export function _useFirestoreRef(
   }
 
   // allow to destructure the returned value
-  Object.defineProperties(data, {
-    error: {
-      get: () => error,
-    },
-    data: {
-      get: () => data,
-    },
-    pending: {
-      get: () => pending,
-    },
-    promise: {
-      get: () => promise,
-    },
-    unbind: {
-      get: () => unbind,
-    },
+  return Object.defineProperties(data as _RefFirestore<unknown>, {
+    error: { get: () => error },
+    data: { get: () => data },
+    pending: { get: () => pending },
+    promise: { get: () => promise },
+    stop: { get: () => stop },
   })
-
-  // no unwrapRef to have a simpler type
-  return data as _RefFirestore<unknown>
 }
 
 export interface UseCollectionOptions extends _UseFirestoreRefOptions {}
@@ -194,7 +180,6 @@ export function useCollection<
   // explicit generic as unknown to allow arbitrary types like numbers or strings
   R extends CollectionReference<unknown> | Query<unknown>
 >(
-  // TODO: add MaybeRef
   collectionRef: _MaybeRef<_Nullable<R>>,
   options?: UseCollectionOptions
 ): _RefFirestore<_InferReferenceType<R>[]>
@@ -224,7 +209,7 @@ export function useCollection<T>(
   }) as _RefFirestore<VueFirestoreQueryData<T>>
 }
 
-// TODO: split document and collection into two different parts
+// TODO: split document and collection into two different files
 
 export interface UseDocumentOptions extends _UseFirestoreRefOptions {}
 

src/firestore/subscribe.ts

@@ -212,14 +212,13 @@ function subscribeToRefs(
   })
 }
 
-// TODO: get rid of the any
 interface CommonBindOptionsParameter {
   // vm: Record<string, any>
-  target: Ref<any>
+  target: Ref<unknown>
   // key: string
   // Override this property in necessary functions
-  resolve: (value: any) => void
-  reject: (error: any) => void
+  resolve: (value: unknown) => void
+  reject: (error: unknown) => void
   ops: OperationsType
 }
 
@@ -318,8 +317,8 @@ export function bindCollection<T = unknown>(
           validDocs[docChanges[i].doc.id] = true
         }
 
-        resolve = ({ id }) => {
-          if (id in validDocs) {
+        resolve = (data) => {
+          if (data && (data as any).id in validDocs) {
             if (++count >= expectedItems) {
               // if wait is true, finally set the array
               if (options.wait) {
@@ -382,8 +381,6 @@ export function bindDocument<T>(
 ) {
   const options = Object.assign({}, DEFAULT_OPTIONS, extraOptions) // fill default values
   const key = 'value'
-  // TODO: warning check if key exists?
-  // const boundRefs = Object.create(null)
 
   const subs: Record<string, FirestoreSubscription> = Object.create(null)
   // bind here the function so it can be resolved anywhere

src/firestore/utils.ts

@@ -23,10 +23,7 @@ export const firestoreDefaultConverter: FirestoreDataConverter<VueFirestoreDocum
     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,
-            },
+            id: { value: snapshot.id },
             // TODO: check if worth adding or should be through an option
             // $meta: {
             //   value: snapshot.metadata,
@@ -38,7 +35,7 @@ export const firestoreDefaultConverter: FirestoreDataConverter<VueFirestoreDocum
   }
 
 export function extractRefs(
-  // FIXME: unknown
+  // TODO: should be unknown instead of DocumentData
   doc: DocumentData,
   oldDoc: DocumentData | void,
   subs: Record<string, { path: string; data: () => DocumentData | null }>
@@ -83,7 +80,9 @@ export function extractRefs(
         // TODO: check and remove
         // Firestore < 4.13
         ref instanceof Date ||
+        // TODO: instanceof Timestamp?
         isTimestamp(ref) ||
+        // TODO: same?
         isGeoPoint(ref)
       ) {
         data[key] = ref

src/index.ts

@@ -1,6 +1,5 @@
 import type { FirebaseApp } from 'firebase/app'
 import type { App } from 'vue'
-import { databasePlugin } from '../dist'
 import { _FirebaseAppInjectionKey } from './app'
 
 // Database