feat(auth): Expose modular API that matches the Firebase web JS SDK v9 API

[Lyokone]

Description

Related issues

#6705

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 18, 2023 11:15pm
react-native-firebase-next	❌ Failed (Inspect)			Sep 18, 2023 11:15pm

[codecov]

Codecov Report

Merging #7015 (8c0a612) into main (acc58da) will decrease coverage by 34.29%.
The diff coverage is 8.29%.

❗ Current head 8c0a612 differs from pull request most recent head 56381a1. Consider uploading reports for the commit 56381a1 to get more accurate results

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #7015       +/-   ##
=============================================
- Coverage     53.58%   19.29%   -34.29%     
+ Complexity      736      286      -450     
=============================================
  Files           235      236        +1     
  Lines         11740    11895      +155     
  Branches       1876     1880        +4     
=============================================
- Hits           6290     2294     -3996     
- Misses         5096     8877     +3781     
- Partials        354      724      +370     

[mikehardy]

just to note passes all tests locally, structurally looks right, I'm running through index.js right now with an eye towards how platform differences were handled (always a point of discussion, but already discussed I think...) and why github is telling me that there was no test coverage on the new modular APIs (when it looks like they were covered... 🤔 )

[mikehardy]

All seems to work, and the cause of coverage notices appears to be that the declaration lines are not covered but the implementation lines are? This seems odd and worth a (separate, not related to this PR) investigation but shouldn't be a blocker. Hypothesis based on looking at this: https://app.codecov.io/gh/invertase/react-native-firebase/pull/7015

I have a couple real questions though:

• can we have only the modular e2e tests, with their non-modular compat testing inside of them, and remove the non-modular tests? Really, just make the modular tests the new default, no need to have modular in the name
• the provider, rnReload, and multifactor tests do not seem to be modularized, if those are not modularized / exercising those features in the v9 / modular way I fear they will break in the future. Unless I am misunderstanding (I could be! please let me know) I think that's a blocker for merge

[Lyokone]

Thanks for the review.

• For the first point, all the other migrated modules have kept the .modular for the new tests. I think we can remove the non-modular of all plugins at the same time but I don't see a reason to do it only for auth.
• For the second point, I missed that you could access multifactor with a top level function now, I'll add the 3 missing e2e tests.

[Lyokone]

This signOut hangs out, I have no idea why @mikehardy

[mikehardy]

Huh - that is unexpected (of course!) - I'll check it locally and see what I can do about CI / investigating

[mikehardy]

For the first point, all the other migrated modules have kept the .modular for the new tests. I think we can remove the non-modular of all plugins at the same time but I don't see a reason to do it only for auth.

Okay - agreed there, I think we did one of them (the most recent?) with just convert-main-file-name-to-modular + compat-inside-modular and I'd like that going forward. You are correct that since there are quite a few in the old style (that this one is using) of old-style + modular-with-compat-inside we can do one PR that realigns all at once. No need to hold this up waiting for that

I'll check the new tests and see what I can find out about signOut hanging. Thanks @Lyokone !

[mikehardy]

Warning: 609:11 warning 'authPhoneMultiFactor' is assigned a value but never used @typescript-eslint/no-unused-vars

☝️ lint failure here @Lyokone

[russellwheatley]

I think a PhoneAuthProvider test needs to be updated. And there's the lint failure that Mike mentioned. Other than that, LGTM 👌.

[mikehardy]

I fixed the PhoneAuthProvider constructor to throw in a backwards-compatible way and cleared the lint error, rebased to current main and re-pushed, we'll see what CI says, seems to run fine locally...

[mikehardy]

Note to self: need to add API for #7148 that came in while this was in-flight

[mikehardy]

rebased to main and re-pushed but signOut likely still hanging, I believe it's a javascript level error, possibly with interaction between rnReload

[mikehardy]

I think everything is fixed now, pending CI

[mikehardy]

Passed all the tests except lint on the last run, I fixed the lint one - merging!

[aedocoding]

Hello, I was wondering if this PR being merged means that the next release of react-native-firebase would have the Config interface accessible to users to make changes to properties like the apiHost or tokenApiHost? Apologies if this is the wrong place to ask this question by the way