react-native-firebase icon indicating copy to clipboard operation
react-native-firebase copied to clipboard

refactor(firestore): improve firestore types and add test

Open ycmjason opened this issue 3 months ago • 7 comments

Description

#8611 flaged that many types are currently broken.

This PR fixes most of the type issues by removing <AppModelType, DbModelType> and replaced with <T extends DocumentData>.

Also added tsd to make sure our .d.ts are legit. Fixed many issues thanks to it!

Related issues

#8611

Release Summary

Fix types for firestore

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • [x] 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__
  • [x] I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • [ ] Yes
    • [x] No

Test Plan

Added tsd to add some type testings to our apis.


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

ycmjason avatar Sep 16 '25 08:09 ycmjason

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 Sep 16, 2025 8:49am

vercel[bot] avatar Sep 16 '25 08:09 vercel[bot]

Hey there, any updates on this? Just got a runtime error as snapshot.exists() has become a method (I've recently upgraded from v21 to v23.3.1) and my code was still treating it as a property, and the types are now broken, so I got no type errors. Thanks!

cbdeveloper avatar Sep 26 '25 03:09 cbdeveloper

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

github-actions[bot] avatar Oct 24 '25 04:10 github-actions[bot]

Hey @mikehardy @ycmjason , any updates on this one? Let me know if I can help somehow. Thank you

cbdeveloper avatar Oct 29 '25 06:10 cbdeveloper

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

github-actions[bot] avatar Nov 26 '25 06:11 github-actions[bot]

I think something bigger is about to happen - we've got a PR landing with long-awaited withConverter support

However, it will affect these firestore types deeply - could I ask an interested party here @ycmjason or @cbdeveloper to have a look at that and see what you think? The implementation there looks right but I'm unsure if it's a breaking change or not. The types appear to have fidelity with upstream firebase-js-sdk so they seem more correct to me but if they are breaking I want to make sure we release a major version with warning and explanation for everyone

And of course if I'm wrong in any of that analysis or assumptions please let me know 🙌

mikehardy avatar Nov 29 '25 02:11 mikehardy

@mikehardy please count me in!

ycmjason avatar Nov 29 '25 07:11 ycmjason