vuejs / vuefire

🔥 Firebase bindings for Vue.js
https://vuefire.vuejs.org
MIT License
3.82k stars 323 forks source link

typescript type definition not consistence: useDocument return null reference if document does not exist #1477

Closed tlserver closed 6 months ago

tlserver commented 6 months ago

Reproduction

codeSandbox

Steps to reproduce the bug

No step required. count: null is displayed on startup.

Expected behavior

In https://github.com/vuejs/vuefire/blob/main/src/firestore/index.ts#L75-L104

export function useDocument<
  // explicit generic as unknown to allow arbitrary types like numbers or strings
  R extends DocumentReference<unknown>
>(
  documentRef: MaybeRefOrGetter<_Nullable<R>>,
  options?: UseDocumentOptions<_InferReferenceType<R>>
): _RefFirestore<_InferReferenceType<R> | undefined> // this one can't be null or should be specified in the converter

/**
 * Creates a reactive collection (usually an array) of documents from a collection ref or a query from Firestore.
 * Accepts a generic to **enforce the type** of the returned Ref. Note you can (and probably should) use
 * `.withConverter()` to have stricter type safe version of a collection reference.
 *
 * @param collectionRef - query or collection
 * @param options - optional options
 */
export function useDocument<T>(
  documentRef: MaybeRefOrGetter<_Nullable<DocumentReference>>,
  options?: UseDocumentOptions<T>
): _RefFirestore<VueFirestoreDocumentData<T> | undefined>

export function useDocument<T>(
  documentRef: MaybeRefOrGetter<_Nullable<DocumentReference<unknown>>>,
  options?: UseDocumentOptions<T>
): _RefFirestore<VueFirestoreDocumentData<T> | undefined> {
  // no unwrapRef to have a simpler type
  return _useFirestoreRef(documentRef, options) as _RefFirestore<
    VueFirestoreDocumentData<T>
  >
}

The return type of useDocument should be _RefFirestore<VueFirestoreDocumentData<T> | null | undefined> but not _RefFirestore<VueFirestoreDocumentData<T> | undefined>, since it return a null reference if document does not exist.

This code, doc.value !== null, should be valid and have no warning.

Actual behavior

IDE warn at doc.value !== null

Additional information

No response

posva commented 6 months ago

FYI VueFirestoreDocumentData already includes null. This is likely related to your converter not being correctly typed. If it can return null, it should be a possibility in withConverter<T>. If typed correctly, it works:

Screenshot 2024-01-03 at 10 40 59

BTW, your codesandbox is read only, when opening issues make sure to share writable playgrounds