Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
tlserver opened this issue Dec 29, 2023 · 1 comment

Comments

@tlserver
Copy link
Contributor

tlserver commented Dec 29, 2023

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
Copy link
Member

posva commented Jan 3, 2024

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

@posva posva closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants