fix(auth): remember authStateFlow(), add e2e coverage for phone linking#2354
fix(auth): remember authStateFlow(), add e2e coverage for phone linking#2354demolaf wants to merge 18 commits into
Conversation
* fix(auth): improve user identifier retrieval * updates
#2322) Co-authored-by: russellwheatley <russellwheatley85@gmail.com>
…o-open (#2326) * fix(auth): update AuthState to reflect success or idle based on user result * updates * updates * refactor: rename auth user state handler
* fix(auth): add configurable params for custom UI - AuthMethodPicker - EmailAuth - PhoneAuth - MFA Enrollment & Challenge * fix(auth): add demo activities for email and phone authentication with customizable UI * updates * updates
…reen (#2330) * feat(auth): add termsContent slot to AuthMethodPicker for custom ToS UI * feat(auth): add termsAccepted parameter to AuthMethodPicker for consent handling * update example * updates * updates * updates
…2319) * feat(auth): add isCredentialLinkingEnabled for authenticated non-anonymous users * t feat(app): add CredentialLinkingDemoActivity to sample app # Conflicts: # app/src/main/AndroidManifest.xml * updates
…rror messages (#2333) * feat(auth): handle GIdP password policy violations with specific error messages * updates * updates * cleanup
* fix(auth): allow saml. prefixed provider IDs in AuthUIConfiguration validation * updates
…2332) * wip # Conflicts: # auth/src/main/java/com/firebase/ui/auth/configuration/auth_provider/EmailAuthProvider+FirebaseAuthUI.kt # auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt * feat(auth): add reauthentication flow with automatic operation retry Adds AuthState.ReauthenticationRequired, withReauth(), and createReauthFlow() to support sensitive operations that require recent sign-in. FirebaseAuthUI.delete() and withReauth() automatically catch FirebaseAuthRecentLoginRequiredException, emit the new state carrying the original operation as retryOperation, and FirebaseAuthScreen presents a ModalBottomSheet overlay scoped to the user's linked providers — no navigation away from the authenticated screen. On successful reauthentication the original operation is retried automatically. * test(auth): add e2e tests for reauthentication flow UI * updates * fix reviews * update README
…on (#2316) * feat(auth): implement legacyFetchSignInWithEmail configuration option * test: write tests for legacyFetchSignInWithEmail * fix(pr): address reviewer feedback on email recovery routing * refactor(pr): address reviewer feedback on legacy email recovery * fix: cancellation callback + test * chore: ignore kotlin cache * chore: run demo app script from command line --------- Co-authored-by: demolaf <demolafadumo@gmail.com>
…string resources (#2339) * feat(auth): add localized loading messages for various sign-in states * updates * feat(auth): update sign-in methods to use AuthUIConfiguration for localized messages * refactor: use periods to represent ellipsis not the unicode character * refactor: use qualifiers, not base values in tests
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes state flow collection in FirebaseAuthScreen, EmailAuthScreen, and PhoneAuthScreen by wrapping authUI.authStateFlow() in a remember(authUI) block to prevent unnecessary flow recreation during recompositions. Additionally, it introduces an integration test to verify that the onSuccess callback fires correctly when a returning signed-in user links a phone number. The review feedback highlights a potential infinite loop in the test's retry logic if emulatorApi.fetchVerifyPhoneCode(phone) returns null instead of throwing an exception, and suggests throwing an exception explicitly to trigger the retry block.
mikehardy
left a comment
mikehardy
left a comment
There was a problem hiding this comment.
looks good - out of curiosity I probed whether authUI was stable, it's a plain class with no equals/hashCode override so it'll get reference identity and be stable ✅
there was a subtle interaction with multi-tenancy or custom stuff - if people use FirebaseAuthUI.create() inside a composable body remember won't have a stable key and the optimization here won't take effect. May be worth adding to the method note with a cross link to this PR for context so people can avoid a footgun https://github.com/firebase/FirebaseUI-Android/blob/version-10.0.0-beta03/auth/src/main/java/com/firebase/ui/auth/FirebaseAuthUI.kt#L667-L668
not a code problem though for sure, no reason not to +1 on my review
demolaf
force-pushed
the
version-10.0.0-beta03
branch
2 times, most recently
from
85acecb to
0eff663
Compare
4 participants
Firebase authentication screens (
PhoneAuthScreen,FirebaseAuthScreen,EmailAuthScreen) callauthUI.authStateFlow().collectAsState(...)directly in the composable body withoutrememberwhich causes unnecessary recompositions.Changes
authUI.authStateFlow()inremember(authUI) { ... }inPhoneAuthScreen.kt,FirebaseAuthScreen.kt, andEmailAuthScreen.kt, so the flow is subscribed once per screen instance instead of on every recomposition.PhoneAuthScreenTest.ktfor a returning, already-signed-in user linking a phone number.