Skip to content

Conversation

supersimple33
Copy link

Description

I have added the withConverter method which is provided in the JS sdk to the package here.

Release Summary

  • Added withConverter to CollectionReference
  • Created new interface FirestoreDataConverter

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes 🔥
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

Copy link

vercel bot commented Aug 23, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
react-native-firebase Ready Ready Preview Comment Aug 23, 2025 5:55pm

@CLAassistant
Copy link

CLAassistant commented Aug 23, 2025

CLA assistant check
All committers have signed the CLA.

@supersimple33 supersimple33 changed the title feat(firestorm): Add data converter feat(firestore): Add data converter Aug 25, 2025
@supersimple33
Copy link
Author

Bump!

@supersimple33
Copy link
Author

Bump!

@macksal
Copy link

macksal commented Sep 11, 2025

Have you noticed that CollectionReference only has one type param, but it's passed two?

export interface CollectionReference<T extends DocumentData = DocumentData> extends Query<T> {

In your method declaration:

withConverter<NewAppModelType, NewDbModelType extends DocumentData = DocumentData>(
  converter: FirestoreDataConverter<NewAppModelType>,
): CollectionReference<NewAppModelType, NewDbModelType>;

It's not just your code, I came across this PR because there's something wrong with these typings already.

@supersimple33
Copy link
Author

supersimple33 commented Sep 12, 2025

Hey @macksal good catch I didn't notice it but you're right the types in this repo aren't yet inline with the firebase sdk since they are lacking the DbModel type im gonna remove that for now since adding it should be its own PR.

Edit: or actually after taking a quick look it may make more sense for me to just add it in now I guess since that type is needed elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants