useFirestoreCollectionData infinite loop when using a Query withConvertor

[bruceharrison1984]

Version info

React: 18.2.0

Firebase: 9.13.0

ReactFire: 4.2.2

Test case

const assignTypes = () => {
  return {
    toFirestore(doc: NewsItemDTO): DocumentData {
      return doc;
    },
    fromFirestore(snapshot: QueryDocumentSnapshot): NewsItemDTO {
      return snapshot.data()! as NewsItemDTO;
    },
  };
};

const NewsPage = () => {
  const firestore = useFirestore();
  const animalsCollection = collection(firestore, 'newsItems');
  const animalsQuery = query(animalsCollection).withConverter(assignTypes());
                                                  ^^^ convertor used here
  const { status, data } = useFirestoreCollectionData(animalsQuery);

  console.log(data);

  if (status === 'loading') {
    return <span>loading...</span>;
  }

  return (
    <>
      <div className="mx-auto lg:w-2/3 xs:w-full">
        <div className="space-y-2">
          <NewsItemList news={data} />
        </div>
      </div>
      <AddNewsButton />
    </>
  );
};

export default NewsPage;

Steps to reproduce

Using useFirestoreCollectionData with a query that has a convertor attached results in an infinite loop of undefined. Removing the withConvertor call from the query immediately fixes the issue. The bug seems to occur whether attaching the convertor to the query or the collection.

Expected behavior

I would expect the query to succeed regardless of if a convertor is utilized or not. Currently, I can only return an untyped collection and then have to map it in an additional step. You can also force the datatype by using an as clause at the point of usage:

<NewsItemList news={data as NewsItemDTO[]} />

but this also feels unnecessary.

Single documents work fine with convertors.

Actual behavior

The query loops forever, returning zero results.

[gtrak]

I tried to use the library for the first time today and hit upon this issue. I think the cause is due to the call to queryEqual here:

reactfire/src/firestore.tsx

Line 16 in 3cb4468

const index = cachedQueries.findIndex((cachedQuery) => queryEqual(cachedQuery, query));

In my case, I am minting new converter objects on each render, which breaks the equality check, with something like:

Firestore.collection(firestore, prefix, 'doc').withConverter(
      docConverter<Doc>()
    ),

If I instead save off the converter to a top-level constant, which means the equality check can presumably rely on identity for the converter part, I can see the collection promise gets fulfilled and my app renders correctly.

It's something like this saved off outside of the component lifecycle, in my case at the top level of a module:

const docConverter = docConverter<Doc>()

Then during a render, it's reusing the object:

Firestore.collection(firestore, prefix, 'doc').withConverter(
      docConverter
    ),

I think this is not a reactfire bug, but I think a common use-case that should be documented as I wasted hours on it and I imagine others would, too.

[gtrak]

Another approach to consider is to allow users to pass in a query key override, like react-query (the lib I was coming from), if it's too difficult to track whether equality will work out from reading subtle code changes.