CSFrequency/react-firebase-hooks

Data Type definition

bartkoedijk opened this issue · 8 comments

bartkoedijk commented

Hi,

I have been using your NPM package for a while now and I love it.
Currently I am using Typescript + React Native for my project.

I came across a Type definition and wondert if was necessary.

const [values, loading, error] = useCollectionData<MyType[]>(ref);

In the line above I would expect values to be a type of: MyType[].
Instead it returned: values: Data<MyType[], "", "">[] | undefined

Is it a possibility to remove the wrapper?

Thank you,
Bart

hannojg commented

Hi @bartkoedijk,

I am currently porting this library over for react-native. I explicitly fixed the problem you are describing in this new library.
https://github.com/skillnation/react-native-firebase-hooks

The library is stable now, however it is currently missing documentation, but the API works 1:1 the same as with this library.

chaosLegacy commented

Hi,
I'm new to firebase, and I found these hooks pretty much useful, but I have a little problem.

I'm using firebase ^9.6.1 along with react-firebase-hooks 4.0.1

This is the piece of code that I use to retreive my messages from firestore
The type Post that i' passing to useCollectionData seems generating an error: see below

    const messagesRef = collection(firestore, 'messages');
    const q = query(messagesRef, orderBy('createdAt'), limit(25));

    const [messages] = useCollectionData<Post[]>(q, { idField: 'id' });

Argument of type 'Query<DocumentData>' is not assignable to parameter of type 'Query<Post[]>'.
  Types of property 'converter' are incompatible.
    Type 'FirestoreDataConverter<DocumentData> | null' is not assignable to type 'FirestoreDataConverter<Post[]> | null'.
      Type 'FirestoreDataConverter<DocumentData>' is not assignable to type 'FirestoreDataConverter<Post[]>'.
        The types returned by 'fromFirestore(...)' are incompatible between these types.
          Type 'DocumentData' is not assignable to type 'Post[]'

Has anyone faced a similar issue?
It seems that someone else had the same issue, it's related to the fact that
with firebase v9 defines its types, so it's no longer possible to cast directly, instead use the built in FirebaseDataConverter
#180 (comment)

I did that

    const postConverter = {
        toFirestore(post: WithFieldValue<Post>): DocumentData {
            debugger
            return { text: post.text, id: post.id };
        },
        fromFirestore(
            snapshot: QueryDocumentSnapshot,
            options: SnapshotOptions
        ): Post {
            debugger
            const data = snapshot.data(options)!;
            return {
                id: data.idField,
                text: data.text,
                uid: data.uid
            }
        }
    };

and I hooked it to my query

    const messagesRef = collection(firestore, 'messages');
        const q = query(messagesRef, orderBy('createdAt'), limit(25)).withConverter(postConverter); // <---- here

    const [messages] = useCollectionData<Post[]>(q, { idField: 'id' });

Now I get an infinite loop in my code saying:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.

Am I doing something wrong ? :/

  • ❤️1
0x80 commented

Here's an approach I think you could use to get around any typing issue in the meantime.

When I started using this library I remember it didn't have generics yet, so there was no way to get a typed document response. I also wanted to have a slightly different type of response. In my application I like to pass documents around as {id: string, data: T, ref: DocumentReference}, which minimizes boilerplate for me. Also, I don't see the benefit of having an error returned (and handled) everywhere. I prefer to have it throw, as query errors should be very rare in my experience (like missing indexes)

So I wrote some basic abstractions that internally call the hooks without types, and but have a typed response.

Here's an example

export function useTypedDocument<T>(
  ref: firebase.firestore.DocumentReference,
): [Document<T> | undefined, boolean] {
  const [snapshot, isLoading, error] = useDocument(ref);

  if (error) {
    throw error;
  }

  const doc = snapshot?.exists
    ? { id: snapshot.id, data: snapshot.data() as T, ref: snapshot.ref }
    : undefined;

  return [doc, isLoading];
}

You could do the same for useCollection or other the hooks.

Even if you want to keep the response the same, you could wrap the hook 1:1 and apply the types you think it should have. Then later remove your abstraction with a find/replace when the library is fixed.

I must admit I haven't tried this with Firebase 9 yet but since you're in control of typing when using an abstraction like this I'm sure it would work.

philldev commented

removing generics from DocumentReference in useDocument.d.ts seems to solve the problem for me.

  • before
export declare const useDocumentData: <T = DocumentData, IDField extends string = "", RefField extends string = "">(docRef?: DocumentReference| null | undefined, options?: DataOptions<T> | undefined) => [Data<T, IDField, RefField> | undefined, boolean, FirestoreError | undefined];
  • after
export declare const useDocumentData: <T = DocumentData, IDField extends string = "", RefField extends string = "">(docRef?: DocumentReference| null | undefined, options?: DataOptions<T> | undefined) => [Data<T, IDField, RefField> | undefined, boolean, FirestoreError | undefined];
wolfib commented

@chaosLegacy I just ran into a very similar issue: typing a firestore query required using a converter and as soon as I tried that I ran into an infinite loop.

What solved the problem for me was moving the definition of the converter out of the React component. I'm guessing that defining the converter in the component causes useIsFirestoreQueryEqual in the firestore hooks to believe it's seeing a new query all the time and therefore triggering a rerender.

  • ❤️4
chrisbianca commented

In the v5 release, these type generics will be removed completely as they are no longer compatible with the underlying Firestore typings. Instead, useDocumentData, useDocumentDataOnce, useCollectionData and useCollectionDataOnce will return a fourth, snapshot parameter to allow underlying access to things like metadata.

@wolfib you will definitely run into problems if you declare the converter in the React component, and it's definitely best practice to move this outside to ensure it's not re-declared on each render. The behaviour shouldn't be data dependent so this shouldn't cause you a problem.

shamoons commented

Does this have type support?