feat(auth, oauth): add native oauth support / no external packages needed#7019
feat(auth, oauth): add native oauth support / no external packages needed#7019Aetherall wants to merge 6 commits intoinvertase:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
working for android, ios coming soon |
|
@cedricboidin has added ios |
|
Rebased on main. I would appreciate some feedback on this PR :) |
|
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 |
|
Sincere apologies for the delay! I'm processing the PR queue now and should have time to review this, I sincerely appreciate the effort and the patience |
mikehardy
left a comment
There was a problem hiding this comment.
This looks great! Happy to hear it's working for you in production that gives me a lot of confidence as I look to merge this.
I left a couple specific comments that are hopefully easy to handle
I rebased to current main and ran the linters so those hoops have already been jumped through
The only thing I can think of besides the specific comments is to add something in docs/auth - might make sense to great make a page there (slicing in to the prev/next links on two of the related pages to get the circular nav links going) ?
Functionally CI should go green now though 🤞 with fully up to date SDKs etc etc
| signInWithPopup(provider: AuthProvider): Promise<UserCredential>; | ||
| signInWithRedirect(provider: AuthProvider): Promise<UserCredential>; |
There was a problem hiding this comment.
Can you document this here, even if briefly? The rnfirebase.io site is built from the typedoc so anything you put here will be seen + used
| String message = exception.getMessage(); | ||
| String invalidEmail = "The email address is badly formatted."; | ||
|
|
||
| System.out.print(exception); |
There was a problem hiding this comment.
Dumping an error to System.out is discouraged, if it is already being propagated up, then I think this could be removed?
|
Looks like another of the test cases needs a modification similar to those already made: |
|
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 |
|
Summer vacation time was busy enough the stale bot got this one, but it's a good looking chunk of work, hoping to get it shepherded through for merge |
|
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 |
|
scanning stale closed queue for unrelated reasons this still looks harvest-able |
|
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 |
38a52cd to
7af280b
Compare
Description
Right now, documentation recommendation for oauth flows is to use the react-native-app-auth package.
What this package do is pretty close to the default behaviour of the firebase native sdk.
With the support of my company and of my teammates from kraaft.co, I had some time allocated to check whether I can bridge the native sdk with react native within react-native-firebase :)
The goals of such integration would be:
Related issues
#349
#3926
Release Summary
Checklist
AndroidiOS-> If anyone knows his swift I'm down for some help :)e2etests added or updated inpackages/\*\*/e2e-> Would like to have some insights with detox and testing the popupjesttests added or updated inpackages/\*\*/__tests__Test Plan
we are deploying this on our staging, and will QA this soon
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter🔥