feat(firestore): V9 modular APIs

[exaby73]

Description

Related issues

Release Summary

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • Yes
• My change supports the following platforms;
  • Android
  • iOS
• 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:

• 👉 Star this repo on GitHub ⭐️
• 👉 Follow React Native Firebase and Invertase on Twitter

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-native-firebase	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 20, 2023 3:36pm
react-native-firebase-next	❌ Failed (Inspect)			Sep 20, 2023 3:36pm

[codecov]

Codecov Report

Merging #7235 (ac4a698) into main (80ac07e) will decrease coverage by 14.80%.
Report is 7 commits behind head on main.
The diff coverage is 59.29%.

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #7235       +/-   ##
=============================================
- Coverage     68.91%   54.10%   -14.80%     
- Complexity        0      738      +738     
=============================================
  Files           137      247      +110     
  Lines          5708    12159     +6451     
  Branches       1224     1897      +673     
=============================================
+ Hits           3933     6578     +2645     
- Misses         1680     5235     +3555     
- Partials         95      346      +251     

[github-actions]

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

[russellwheatley]

Looking good, @exaby73 👍 . I've provided some feedback!

[mikehardy]

Looks to have some lint errors from local test modifications checked in ? Hopefully a quick fix

[mikehardy]

@exaby73 are you running these locally and they are passing?

  1) firestore().collection().where(OR Filters)
       modular
         returns with where ">" filter:

      AssertionError: expected 12 to equal 2
      + expected - actual

      -12
      +2
      
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at Context.<anonymous> (/Users/runner/work/react-native-firebase/react-native-firebase/packages/firestore/e2e/Query/where.or.filter.e2e.js:1647:28)

[exaby73]

@mikehardy Not all the time. I figured the tests are flaky which is why I'm rerunning it. I'm rerunning it locally now and trying to debug this

[russellwheatley]

LGTM - code coverage checks aside 👍

[mikehardy]

@russellwheatley I'm tracking the code coverage thing separately so I do not forget but it doesn't block this (and the previous ones) from going in --> #7328

[mikehardy]

Alright then - I didn't see anything on a couple scans so (gulp...) here we go!
@exaby73 this is an incredible work, seriously, one of the biggest PRs I've seen in years. Anyone using this package and it's v9 APIs over the years to come will owe you a debt of gratitude

[gregfenton]

Does onCompletion() not get called when the returned unsubscribe() is called?

Or maybe it should be called then?

[mikehardy]

interesting question! Honestly I'm not sure - you'd have to chase in to the native code I think and see what events are passed native<->javascript

Alternatively, if nothing is provided then it does seem reasonable to call onCompletion when unsubscribe happens, as by definition that would mean there will never be more events from that subscription...

[gregfenton]

Hmmm....not promising for Android: https://github.com/search?q=repo%3Afirebase%2Ffirebase-android-sdk%20oncompletion&type=code

Though apparently it is a bigger deal for iOS? https://github.com/search?q=repo%3Afirebase%2Ffirebase-ios-sdk+oncompletion&type=code

Or am I looking in the wrong place entirely?

[mikehardy]

I just looked more closely at the firebase-js-sdk documentation, which we generally treat as our authority here with regard to what behavior contract to meet for APIs, and it's pretty clear (and is the source of that comment I think):

https://firebase.google.com/docs/reference/js/firestore_.md#onsnapshot

NOTE: Although an onCompletion callback can be provided, it will never be called because the snapshot stream is never-ending.

I do not think we can call the onCompletion callback and meet the behavior contract of onSnapshot then, so I don't think this is a productive path to go down with regard to investigation. Sorry I started down it with you, if that meant you spent any significant amount of time on it