Conversation
There was a problem hiding this comment.
Pull request overview
Adds optional authentication callbacks to RxSignInView so integrators can react to sign-in success/failure from SwiftUI, and documents the new API in the README.
Changes:
- Add
onAuthSuccess/onAuthFailedcallback parameters to bothRxSignInViewinitializers. - Invoke callbacks after
manager.authenticate()completes or throws. - Update README usage examples to show callback-based integration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Sources/RxAuthSwiftUI/RxSignInView.swift | Adds callback properties/initializers and calls them from the sign-in button action. |
| README.md | Documents new callback parameters with usage examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Task { | ||
| do { | ||
| try await manager.authenticate() | ||
| onAuthSuccess?() | ||
| } catch { | ||
| // Error is handled by the manager | ||
| onAuthFailed?(error) | ||
| } |
There was a problem hiding this comment.
The Task { ... } created here is not guaranteed to run on MainActor. After await manager.authenticate() returns, execution can resume off-main, so onAuthSuccess/onAuthFailed may be invoked on a background executor (problematic for UI/navigation work). Make the task main-actor isolated (e.g., Task { @MainActor in ... }) or dispatch the callback invocations back to MainActor.
| public init( | ||
| manager: OAuthManager, | ||
| appearance: RxSignInAppearance = RxSignInAppearance() | ||
| appearance: RxSignInAppearance = RxSignInAppearance(), | ||
| onAuthSuccess: (() -> Void)? = nil, | ||
| onAuthFailed: ((Error) -> Void)? = nil | ||
| ) where Header == Never { |
There was a problem hiding this comment.
onAuthSuccess/onAuthFailed are stored on the view, so the corresponding initializer parameters need to be @escaping (otherwise this won’t compile under Swift’s non-escaping default). Given Swift 6 strict concurrency, also consider annotating these callback types as @MainActor/@Sendable so they can be safely captured from the Task used by the button action.
| public init( | ||
| manager: OAuthManager, | ||
| appearance: RxSignInAppearance = RxSignInAppearance(), | ||
| onAuthSuccess: (() -> Void)? = nil, | ||
| onAuthFailed: ((Error) -> Void)? = nil, | ||
| @ViewBuilder header: () -> Header |
There was a problem hiding this comment.
Same issue as the simple init: onAuthSuccess/onAuthFailed are stored properties, so these parameters should be @escaping (and ideally @MainActor/@Sendable in Swift 6) to avoid compile-time escaping/sendability errors when captured by Task.
1 participant
No description provided.