sbatson5 / firestore-jest-mock

Jest Helper library for mocking Cloud Firestore
https://www.npmjs.com/package/firestore-jest-mock
178 stars 58 forks source link

Support withConverter #77

Closed appsbytom closed 3 years ago

appsbytom commented 3 years ago

Summary

In firebase a Query, CollectionReferenceand DocumentReferencecan use the withConverter method, this is not currently part of the mocks or faked query, document or collection

Basic example

const userConverter = {
  toFirestore({ permission }) {
    return { permission };
  },
  fromFirestore(snapshot, options) {
    const { permission } = snapshot.data(options);
    return new User(permission);
  }
};

firestore.collection('users').withConverter(userConverter).get();

Motivation

The above example throws an error firestore.collection(...).withConverter is not a function as there is no withConverter implemented into the faked query, document or collection. I use these converters to avoid duplication of conversions but cannot use if testing with this library.

appsbytom commented 3 years ago

I started to look into this but the way the query, document and collections work had me questioning if I was implementing it correctly so opening an issue to start a dialog

appsbytom commented 3 years ago

I have a PR for this which works for collections however a document does not, none of the other methods provided by a query work either. I expect this is as using queries such as limit or where on a document will not change the result however doc('characters/bob/family') can be used instead of a chain of collection('characters').doc('bob').collection('family') and queries like withConverter are used on single documents

sbatson5 commented 3 years ago

Thanks for posting this. Another method I wasn't familiar with.

I see what you're saying about the differences. It took me a little digging through the documentation to find an example of the function being called on a doc directly:

https://github.com/firebase/firebase-js-sdk/blob/0dfef5dd58f961208e29e249f5e8f5a3e2908c88/packages/firestore/test/integration/api/batch_writes.test.ts#L354-L368

I think trying to reproduce this in one our tests can be a good jumping off point to get it working as expected.

appsbytom commented 3 years ago

Released v0.10.0

triplef commented 8 months ago

Could you please clarify the status of converter support? While I can see that withConverter is mocked, I don’t see the fromFirestore/toFirestore to be actually called as part of getting/setting documents.