refactor(auth): scope phone verification loading state to the composition that owns it - #2454
refactor(auth): scope phone verification loading state to the composition that owns it#2454demolaf wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request simplifies state management in FirebaseAuthUI by removing the AtomicLong revision tracking and instead handling the retraction of the Loading state directly within the UI layer using Compose's lifecycle. Specifically, a DisposableEffect is introduced in PhoneAuthScreen to reset the authentication state to Idle upon disposal if it was left in a Loading state. The review feedback points out a potential bug where rememberUpdatedState(authState) combined with DisposableEffect(authUI) can cause a mismatched state capture when authUI changes, as well as a race condition with cancellation. A code suggestion is provided to track the state per authUI instance using remember(authUI) { mutableStateOf(authState) }.
FirebaseAuthUIcarried a revision counter plusclearLoadingStateandcurrentAuthStateRevisionpurely soverifyPhoneNumbercould retract its ownAuthState.Loadingon cancellation without clobbering whatever superseded it.verifyPhoneNumbernow writes no state on cancellation and just rethrows, andPhoneAuthScreenretracts its ownLoadinginstead — inonChangeNumberClick, and from aDisposableEffectwhen the composition goes away mid-verification. Cancelling an attempt was already the screen's own bookkeeping, so the state it leaves behind belongs to the screen too.Dropped three tests covering the removed API and the old callee-retracts contract, and added one to
PhoneAuthScreenVerificationLifecycleTestfor the change-number case — verified it fails without the change.