Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ fun SignUpUI(
val isFormValid = remember(displayName, email, password, confirmPassword) {
derivedStateOf {
listOf(
displayNameValidator.validate(displayName),
!provider.isDisplayNameRequired || displayNameValidator.validate(displayName),
Comment on lines 95 to +98

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Unnecessary derivedStateOf and Side-Effects during Composition

There are two main issues with the current implementation of isFormValid:

  1. derivedStateOf Anti-Pattern:
    derivedStateOf is designed to derive state from other Compose State objects (like MutableState). Here, the inputs (displayName, email, password, confirmPassword) are plain String parameters, not Compose States. Because they are plain values, Compose cannot track them inside the derivedStateOf block. To make it work, they are passed as keys to remember. However, when any of these keys change, the entire remember block is re-executed, creating a brand new derivedStateOf instance on every keystroke. This completely defeats the purpose of derivedStateOf and adds unnecessary allocation and registration overhead.

  2. Side-Effects during Composition:
    Inside the derivedStateOf block, validate() is called on the validators (e.g., displayNameValidator.validate(displayName)). The validate() function is not pure; it mutates the internal state (_validationStatus) of the validator. Running side-effects inside a composition/calculation block is highly discouraged in Compose as it can lead to inconsistent UI states or unexpected behavior.

Recommended Solution

Simplify isFormValid to a plain Boolean using remember with keys, and update its usage in the Button to remove .value:

// 1. Simplify isFormValid to a plain Boolean
val isFormValid = remember(displayName, email, password, confirmPassword) {
    listOf(
        !provider.isDisplayNameRequired || displayNameValidator.validate(displayName),
        emailValidator.validate(email),
        passwordValidator.validate(password),
        confirmPasswordValidator.validate(confirmPassword)
    ).all { it }
}

// 2. Update the Button's enabled state (around line 203) to use isFormValid directly:
Button(
    onClick = { onSignUpClick() },
    enabled = !isLoading && isFormValid, // Removed .value
) {
    // ...
}

emailValidator.validate(email),
passwordValidator.validate(password),
confirmPasswordValidator.validate(confirmPassword)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* Copyright 2025 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the
* License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.firebase.ui.auth.ui.screens.email

import android.content.Context
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.test.assertIsEnabled
import androidx.compose.ui.test.hasClickAction
import androidx.compose.ui.test.hasText
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performTextInput
import androidx.test.core.app.ApplicationProvider
import com.firebase.ui.auth.configuration.authUIConfiguration
import com.firebase.ui.auth.configuration.auth_provider.AuthProvider
import com.firebase.ui.auth.configuration.string_provider.AuthUIStringProvider
import com.firebase.ui.auth.configuration.string_provider.DefaultAuthUIStringProvider
import com.firebase.ui.auth.configuration.string_provider.LocalAuthUIStringProvider
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config

/**
* Unit tests for [SignUpUI], covering form validity logic.
*
* @suppress Internal test class
*/
@Config(sdk = [34])
@RunWith(RobolectricTestRunner::class)
class SignUpUITest {

@get:Rule
val composeTestRule = createComposeRule()

private lateinit var applicationContext: Context
private lateinit var stringProvider: AuthUIStringProvider

@Before
fun setUp() {
applicationContext = ApplicationProvider.getApplicationContext()
stringProvider = DefaultAuthUIStringProvider(applicationContext)
}

@Test
fun `sign up button becomes enabled when display name is not required and hidden`() {
val provider = AuthProvider.Email(
isDisplayNameRequired = false,
emailLinkActionCodeSettings = null,
passwordValidationRules = emptyList()
)
val configuration = authUIConfiguration {
context = applicationContext
providers { provider(provider) }
}

composeTestRule.setContent {
CompositionLocalProvider(LocalAuthUIStringProvider provides stringProvider) {
var email by remember { mutableStateOf("") }
var password by remember { mutableStateOf("") }
var confirmPassword by remember { mutableStateOf("") }

SignUpUI(
configuration = configuration,
isLoading = false,
displayName = "",
email = email,
password = password,
confirmPassword = confirmPassword,
onDisplayNameChange = { },
onEmailChange = { email = it },
onPasswordChange = { password = it },
onConfirmPasswordChange = { confirmPassword = it },
onGoToSignIn = { },
onSignUpClick = { }
)
}
}

// Name field should not be rendered since it isn't required.
composeTestRule.onNodeWithText(stringProvider.nameHint).assertDoesNotExist()

composeTestRule.onNodeWithText(stringProvider.emailHint)
.performTextInput("test@example.com")
composeTestRule.onNodeWithText(stringProvider.passwordHint)
.performTextInput("Password123")
composeTestRule.onNodeWithText(stringProvider.confirmPasswordHint)
.performTextInput("Password123")

composeTestRule.waitForIdle()

// With email/password/confirmPassword all valid and no display name required,
// the sign up button should be enabled even though displayName is still "".
composeTestRule.onNode(hasText(stringProvider.signupPageTitle.uppercase()) and hasClickAction())
.assertIsEnabled()
}
}
Loading