Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the GETTING_STARTED.md documentation to align with modern Android development standards, including Jetpack Compose, Kotlin 2.0+, and reactive state collection. Key improvements include extracting @composable theme getters from non-inline lambdas and demonstrating the proper lifecycle management of AuthFlowController in Activities. Feedback focuses on optimizing Compose performance by wrapping authUIConfiguration and the rememberAuthConfiguration helper in remember blocks to avoid unnecessary recompositions, as well as guarding the Activity-based intent launch with a savedInstanceState == null check to prevent relaunching on configuration changes.
| val configuration = authUIConfiguration { | ||
| context = applicationContext | ||
| theme = AuthUITheme.fromMaterialTheme() | ||
| theme = authTheme |
There was a problem hiding this comment.
The configuration is created directly inside the setContent block without being wrapped in remember. Since AuthUIConfiguration is an unstable type, creating a new instance on every recomposition will cause FirebaseAuthScreen to recompose unnecessarily. It is highly recommended to wrap the authUIConfiguration block in remember(authTheme) to cache the configuration across recompositions.
| return authUIConfiguration { | ||
| context = appContext | ||
| providers { | ||
| provider(AuthProvider.Email()) | ||
| provider(AuthProvider.Google(scopes = listOf("email"), serverClientId = null)) | ||
| } | ||
| theme = authTheme | ||
| } |
There was a problem hiding this comment.
The helper function is named rememberAuthConfiguration, but it does not actually use Compose's remember API. Without remember, a new AuthUIConfiguration instance will be created on every recomposition, which can trigger unnecessary recompositions in child composables (like FirebaseAuthScreen) because AuthUIConfiguration is not a stable type. Wrapping the configuration builder in remember ensures it is cached across recompositions.
| return authUIConfiguration { | |
| context = appContext | |
| providers { | |
| provider(AuthProvider.Email()) | |
| provider(AuthProvider.Google(scopes = listOf("email"), serverClientId = null)) | |
| } | |
| theme = authTheme | |
| } | |
| return remember(appContext, authTheme) { | |
| authUIConfiguration { | |
| context = appContext | |
| providers { | |
| provider(AuthProvider.Email()) | |
| provider(AuthProvider.Google(scopes = listOf("email"), serverClientId = null)) | |
| } | |
| theme = authTheme | |
| } | |
| } |
| authController = FirebaseAuthUI.getInstance().createAuthFlow(configuration) | ||
| authLauncher.launch(authController.createIntent(this)) |
There was a problem hiding this comment.
Launching the activity result launcher directly in onCreate without checking savedInstanceState == null will cause the authentication flow to be launched again whenever the activity is recreated (such as during a device rotation). Checking if savedInstanceState is null ensures that the flow is only launched on the initial creation of the activity.
| authController = FirebaseAuthUI.getInstance().createAuthFlow(configuration) | |
| authLauncher.launch(authController.createIntent(this)) | |
| authController = FirebaseAuthUI.getInstance().createAuthFlow(configuration) | |
| if (savedInstanceState == null) { | |
| authLauncher.launch(authController.createIntent(this)) | |
| } |
e6e4aba to
e1ac97f
Compare
|
Folded into #2544. The open review threads are addressed there: |
Four defects in this guide stop a reader who copies it from compiling, and the install block never mentions Compose while every snippet calls
setContent.Changes
AuthUITheme.fromMaterialTheme()andAuthUITheme.AdaptiveaboveauthUIConfiguration { }, which is a plainfun, notinline, so neither@Composableread compiles inside it.com.facebook.android:facebook-login:18.3.0, which is what the build resolves.facebook-android-sdkdoes not exist.compose-bom,compose-uiandmaterial3to the install block, since:authdeclares its own Compose dependencies asimplementation.auth/README.md.isSignedIn()gate withauthStateFlow().collectAsState.isSignedIn()is a plaincurrentUser != nullread with no snapshot backing, so the UI never left the auth screen.AuthFlowControlleras a field, dispose it inonDestroy(), and register the launcher as a property initializer rather than afterSTARTED.No library version is touched: the
10.0.0-beta05pin and the beta banner belong to a separate doc pass.Maintainer note: Fixes internal CPRN-616