Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make AuthInterop and AppCheckInterop properties optional in Storage #12387

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

andrewheard
Copy link
Contributor

The AuthInterop and AppCheckInterop instances returned by ComponentType<ProtocolName>.instance(for: ProtocolName.self, in: app.container) are nullable (e.g., if AppCheck or Auth aren't added to an app). This hasn't broken the Storage SDK because the instances are only used in StorageTokenAuthorizer, where they're nullable parameters.

#no-changelog

@andrewheard
Copy link
Contributor Author

Note: We could consider making the method FIRComponentType
+ (T)instanceForProtocol:inContainer: return a nullable T instead.

This would force us to ensure we handle nil in cases like AuthInterop or AppCheckInterop but would require us to force-unwrap in cases like StorageProvider where we know the value won't be nil (or something is very wrong if it is).

@andrewheard andrewheard marked this pull request as ready for review February 13, 2024 22:55
@andrewheard andrewheard requested a review from paulb777 February 13, 2024 22:55
@andrewheard andrewheard merged commit ab0d085 into main Feb 14, 2024
55 checks passed
@andrewheard andrewheard deleted the ah/storage-interop-nullability branch February 14, 2024 15:03
@paulb777
Copy link
Member

@andrewheard Yes, I think it should return a nullable T.

Instead of force unwrapping, we should do if let ... else fatalError()

@firebase firebase locked and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants