Sean/mobile 464 ios add public api to clear pre filled email#381
Conversation
Introduce three new view modifiers on AuthView: - clerkInitialIdentifier(_:) to pre-fill email/username - clerkInitialPhoneNumber(_:) to pre-fill phone number - clerkPersistsIdentifiers(_:) to control persistence When persistence is disabled, previously stored identifiers are cleared and future edits are not saved. This supports shared device scenarios where user data should not persist.
When providing an initial identifier or phone number, clear the opposite stored field to prevent stale data from appearing in the form. Also move identifier config modifiers to View extension for broader usability.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAuth identifier prefill and persistence were made configurable: new iOS-only Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant AuthView as AuthView
participant AuthState as AuthState
participant UD as UserDefaults
participant LastUsed as LastUsedAuth
participant AuthStart as AuthStartView
User->>AuthView: apply modifier (initialIdentifier / persistsIdentifiers)
AuthView->>AuthState: .configure(AuthIdentifierConfig) %% apply config on change
AuthState->>AuthState: set hasInitialValues, persistsIdentifiers
alt persistsIdentifiers == true
AuthState->>UD: read identifier/phone (identifierStorageKey / phoneNumberStorageKey)
AuthState->>LastUsed: storeLastUsedIdentifierType(..., userDefaults: UD)
else persistsIdentifiers == false
AuthState->>UD: remove stored identifier/phone
AuthState->>LastUsed: clearStoredIdentifierType(userDefaults: UD)
end
User->>AuthStart: view appears
AuthStart->>AuthState: query lastUsedAuth / hasInitialValues -> update UI fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ClerkKitUI/Components/Auth/AuthStartView.swift (1)
377-386:⚠️ Potential issue | 🟠 MajorMissing
userDefaultsparameter instoreIdentifierTypecalls breaks DI pattern.
AuthStateuses an injecteduserDefaultsinstance, andAuthState.configure()correctly passes it toLastUsedAuth.clearStoredIdentifierType(userDefaults:). However, the calls here use the default.standard, causing writes to potentially go to a differentUserDefaultssuite than reads.Additionally,
authState.persistsIdentifiers(line 378) may not reflect the configured value yet due to SwiftUI's bottom-uponAppearordering—AuthView.onFirstAppear(which callsconfigure()) runs afterAuthStartView.onFirstAppear. Consider using the environment valuepersistsIdentifiersdirectly here for consistency with line 183.🔧 Proposed fix
Since
AuthState.userDefaultsis private, you may need to expose a method onAuthStatefor storing identifier type, or pass the environment value directly:private func storeIdentifierType() { - guard authState.persistsIdentifiers else { return } + guard persistsIdentifiers else { return } if phoneNumberFieldIsActive, phoneNumberIsEnabled { - LastUsedAuth.storeIdentifierType(.phone) + // Consider adding a method on AuthState that wraps this + // or exposing userDefaults access + LastUsedAuth.storeIdentifierType(.phone) } else if authState.authStartIdentifier.isEmailAddress { - LastUsedAuth.storeIdentifierType(.email) + LastUsedAuth.storeIdentifierType(.email) } else { - LastUsedAuth.storeIdentifierType(.username) + LastUsedAuth.storeIdentifierType(.username) } }A cleaner approach would be to add a
storeIdentifierType(_:)method onAuthStatethat uses its internaluserDefaults:// In AuthState.swift func storeLastUsedIdentifierType(_ identifier: LastUsedAuth) { guard persistsIdentifiers else { return } LastUsedAuth.storeIdentifierType(identifier, userDefaults: userDefaults) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift` around lines 377 - 386, The current storeIdentifierType() in AuthStartView calls LastUsedAuth.storeIdentifierType(...) without passing AuthState's injected userDefaults and also reads authState.persistsIdentifiers which may not be initialized yet; add a method on AuthState (e.g., storeLastUsedIdentifierType(_:) or storeIdentifierType(_:)) that checks persistsIdentifiers and writes via its internal userDefaults, then update AuthStartView.storeIdentifierType() to call that new AuthState method (or, alternatively, read the environment persistsIdentifiers instead of authState.persistsIdentifiers) so writes use the correct UserDefaults suite and respect DI.
🧹 Nitpick comments (1)
Sources/ClerkKitUI/Components/Auth/AuthStartView.swift (1)
182-191: Consider using consistent source forlastUsedAuthinitialization.Line 184 initializes
lastUsedAuthwithout passing theuserDefaultsinstance, soretrieveStoredIdentifierType()insideLastUsedAuth.initwill read from.standard. IfAuthStateis configured with a customUserDefaultssuite (e.g., in tests), this could read stale or inconsistent data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift` around lines 182 - 191, Initialize lastUsedAuth with the same UserDefaults instance used by your AuthState to avoid reading from .standard; replace the current LastUsedAuth(environment: Clerk.shared.environment) call with one that passes the AuthState's userDefaults (e.g., LastUsedAuth(environment: Clerk.shared.environment, userDefaults: authState.userDefaults) or the actual variable name for your AuthState's UserDefaults), so retrieveStoredIdentifierType() reads from the correct suite instead of .standard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift`:
- Around line 377-386: The current storeIdentifierType() in AuthStartView calls
LastUsedAuth.storeIdentifierType(...) without passing AuthState's injected
userDefaults and also reads authState.persistsIdentifiers which may not be
initialized yet; add a method on AuthState (e.g.,
storeLastUsedIdentifierType(_:) or storeIdentifierType(_:)) that checks
persistsIdentifiers and writes via its internal userDefaults, then update
AuthStartView.storeIdentifierType() to call that new AuthState method (or,
alternatively, read the environment persistsIdentifiers instead of
authState.persistsIdentifiers) so writes use the correct UserDefaults suite and
respect DI.
---
Nitpick comments:
In `@Sources/ClerkKitUI/Components/Auth/AuthStartView.swift`:
- Around line 182-191: Initialize lastUsedAuth with the same UserDefaults
instance used by your AuthState to avoid reading from .standard; replace the
current LastUsedAuth(environment: Clerk.shared.environment) call with one that
passes the AuthState's userDefaults (e.g., LastUsedAuth(environment:
Clerk.shared.environment, userDefaults: authState.userDefaults) or the actual
variable name for your AuthState's UserDefaults), so
retrieveStoredIdentifierType() reads from the correct suite instead of
.standard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7cba716a-e4e7-470e-897d-80d305389683
📒 Files selected for processing (6)
Sources/ClerkKitUI/Components/Auth/AuthStartView.swiftSources/ClerkKitUI/Components/Auth/AuthState.swiftSources/ClerkKitUI/Components/Auth/AuthView.swiftSources/ClerkKitUI/Components/Auth/LastUsedAuth.swiftSources/ClerkKitUI/Extensions/View+AuthIdentifierConfig.swiftTests/UI/AuthStateConfigurationTests.swift
Move the `persistsIdentifiers` guard check into `AuthState.storeLastUsedIdentifierType()` to centralize persistence logic and simplify the call sites in AuthStartView.
This replaces the generic View extension modifiers (clerkInitialIdentifier, clerkInitialPhoneNumber, clerkPersistsIdentifiers) with type-specific methods directly on AuthView (initialIdentifier, initialPhoneNumber, persistsIdentifiers). The configuration is now stored in an AuthIdentifierConfig struct and passed through the view hierarchy rather than using SwiftUI environment values.
Merge initialIdentifier and initialPhoneNumber into one method that auto-detects phone numbers vs email/username inputs.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/ClerkKitUI/Components/Auth/AuthView.swift (1)
226-233: Keep identifier fields mutually exclusive when overriding.At Line 227, the method copies existing config, but Lines 228-232 only set one field and never clear the other. If
initialIdentifier(_:)is applied multiple times (email/username then phone, or vice versa), stale values can coexist.Proposed fix
public func initialIdentifier(_ identifier: String) -> AuthView { var config = config + config.initialIdentifier = nil + config.initialPhoneNumber = nil if identifier.isPhoneNumber { config.initialPhoneNumber = identifier } else { config.initialIdentifier = identifier } return AuthView(mode: authState.mode, isDismissable: isDismissable, config: config) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/Auth/AuthView.swift` around lines 226 - 233, The method initialIdentifier(_ identifier: String) in AuthView currently sets config.initialPhoneNumber or config.initialIdentifier but never clears the other field, allowing stale values to coexist; update initialIdentifier(_:) to explicitly clear the alternate field whenever one is set (i.e., when setting config.initialPhoneNumber also set config.initialIdentifier = nil/empty, and vice versa) before returning the new AuthView via AuthView(mode:isDismissable:config:), ensuring the identifier fields remain mutually exclusive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/ClerkKitUI/Components/Auth/AuthView.swift`:
- Around line 226-233: The method initialIdentifier(_ identifier: String) in
AuthView currently sets config.initialPhoneNumber or config.initialIdentifier
but never clears the other field, allowing stale values to coexist; update
initialIdentifier(_:) to explicitly clear the alternate field whenever one is
set (i.e., when setting config.initialPhoneNumber also set
config.initialIdentifier = nil/empty, and vice versa) before returning the new
AuthView via AuthView(mode:isDismissable:config:), ensuring the identifier
fields remain mutually exclusive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8a4e78dc-a7b4-4b18-a851-f2ce41effce3
📒 Files selected for processing (1)
Sources/ClerkKitUI/Components/Auth/AuthView.swift
Move phone number detection from AuthView to AuthState so that a single initialIdentifier property handles both email/username and phone number inputs. This simplifies the public API while preserving the behavior of clearing the alternate field when seeding a value.
|
I found one iOS-specific issue in the new
The result is that values that look like phone numbers but do not pass full validation get routed into I reproduced this locally on the current PR head by running the iOS test target:
That fails in I think the root issue is that we are using strict phone validation to decide which field to prefill. This probably wants a looser "phone-like" heuristic, or a dedicated phone-prefill path, rather than requiring the value to already be a fully valid number. One reason this is easy to miss: the PR checks are green because CI is currently running the macOS unit-test job, while this test is under |
Use NSDataDetector-based `looksLikePhoneNumber` for determining identifier type during initial setup, keeping the strict PhoneNumberKit validation for other use cases.
seanperez29
deleted the
sean/mobile-464-ios-add-public-api-to-clear-pre-filled-email
branch
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests