Skip to content

refactor(auth): scope phone verification loading state to the composition that owns it - #2454

Draft
demolaf wants to merge 2 commits into
version-10.0.0-beta04from
fix/phone-auth-loading-state-ownership
Draft

refactor(auth): scope phone verification loading state to the composition that owns it#2454
demolaf wants to merge 2 commits into
version-10.0.0-beta04from
fix/phone-auth-loading-state-ownership

Conversation

@demolaf

@demolaf demolaf commented Aug 25, 2026

Copy link
Copy Markdown
Member

FirebaseAuthUI carried a revision counter plus clearLoadingState and currentAuthStateRevision purely so verifyPhoneNumber could retract its own AuthState.Loading on cancellation without clobbering whatever superseded it.

verifyPhoneNumber now writes no state on cancellation and just rethrows, and PhoneAuthScreen retracts its own Loading instead — in onChangeNumberClick, and from a DisposableEffect when 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 PhoneAuthScreenVerificationLifecycleTest for the change-number case — verified it fails without the change.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) }.

Comment thread auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt Outdated
@demolaf
demolaf marked this pull request as ready for review August 25, 2026 09:09
@demolaf
demolaf marked this pull request as draft August 25, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant