Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates dependency version ranges in Package.swift and Package.resolved, adds Sendable conformances to authentication context types, removes redundant await keywords and argument labels, and adds .macOS(.v12) to the supported platforms in Package.swift. Feedback on the changes highlights that declaring macOS support in Package.swift will cause compilation failures because several targets use iOS-only APIs without conditional compilation guards.
| platforms: [ | ||
| .iOS(.v17), | ||
| // Satisfies SPM's dependency graph check for FirebaseAuth 13 & GoogleSignIn 11. | ||
| // This package only supports iOS. | ||
| .macOS(.v12) | ||
| ], |
There was a problem hiding this comment.
Adding .macOS(.v12) to the supported platforms will cause compilation failures when building for macOS. Several targets in this package (such as FirebaseAuthSwiftUI, FirebaseGoogleSwiftUI, and FirebaseStorageUI) contain iOS-only APIs (like UIApplication, UIWindowScene, and UIImage) and do not have #if os(iOS) or #if canImport(UIKit) guards. If this package must declare macOS support to satisfy dependency graph checks, you will need to wrap all iOS-specific source files and APIs in #if os(iOS) guards to prevent compilation errors on macOS. Otherwise, it is highly recommended to revert this change and have consuming applications conditionally depend on this package only for their iOS targets.
platforms: [.iOS(.v17)],There was a problem hiding this comment.
• SPM's Fallback Behavior: When a platform is omitted from platforms: [...] in Package.swift, SPM does not interpret it as "this platform is unsupported". Instead, it falls back to the default minimum
version for that platform based on your swift-tools-version. For tools 6.0, that default is macOS 10.13.
• Dependency Graph Floor Check: SPM enforces that a target's deployment target must be ≥ its dependencies' deployment targets across all platforms SPM thinks the target supports.
• GoogleSignIn requires macOS 12.0 (and FirebaseAuth requires macOS 11.0 in Firebase 13).
• Because FirebaseUI only listed .iOS(.v17), SPM assigned it the default macOS 10.13, saw 10.13 < 12.0, and failed graph validation before building.
• Declaring .macOS(.v12) explicitly raises SPM's macOS floor above GoogleSignIn's requirement so graph validation passes.
There was a problem hiding this comment.
This library didn't work on macOS before this PR, so there's no difference in outcome.
| _ = try await handleAutoUpgradeAnonymousUser(credentials: credential) | ||
| } else { | ||
| let result = try await auth.signIn(withEmail: email, link: link) | ||
| _ = try await auth.signIn(withEmail: email, link: link) |
There was a problem hiding this comment.
No action required, but maybe we could annotate these with allow-unused-result upstream.
paulb777
left a comment
paulb777
left a comment
There was a problem hiding this comment.
Follow-Up Note (Post-Firebase 13 / GSI 10.1.0 Release)
- Once
firebase-ios-sdk13.0.0 andGoogleSignIn-iOS10.1.0 are tagged, consider bumping at least one sample/test Xcode project (samples/swift/FirebaseUI-demo-swift.xcodeprojis currentlyupToNextMajorVersionfrom12.14.0, ande2eTest/FirebaseSwiftUIExample.xcodeprojisupToNextMajorVersionfrom11.12.0) and/orPackage.resolvedso CI builds and tests against Firebase 13 directly.
| platforms: [.iOS(.v17)], | ||
| platforms: [ | ||
| .iOS(.v17), | ||
| // Satisfies SPM's dependency graph check for FirebaseAuth 13 & GoogleSignIn 11. |
|
|
||
| private extension AuthService { | ||
| internal func updateAuthenticationState() { | ||
| extension AuthService { |
There was a problem hiding this comment.
From the agent
Note (Verified Safe, Optional Cleanup): Checked all remaining properties and methods inside this extension AuthService block (lines 918–1190) — every one explicitly declares private, so removing private from extension does not accidentally expose any private helper as internal. However, moving func updateAuthenticationState() out above private extension AuthService would keep private extension AuthService intact so any future helper added under // MARK: - Private Helper Methods stays private by default.
3 participants
After merging, this would need a SPM-only v16.2.0 tag and release page creation.
Usefulness is blocked until GSI 10.1.0 that will include Firebase 13 compatibility: google/GoogleSignIn-iOS#626
CocoaPods should no longer be published in alignment with firebase-ios-sdk.