firebase / firebase-ios-sdk

Firebase SDK for Apple App Development
https://firebase.google.com
Apache License 2.0
5.67k stars 1.49k forks source link

[Auth] Mark *Credential classes as Sendable #14100

Closed ncooke3 closed 3 days ago

ncooke3 commented 3 days ago

These classes have no mutable state so they should meet the semantic requirements of Sendable. In Firebase 12, we should refactor to checked Sendable. We can't currently do so without a breaking change because of the fact that the classes are open (Sendable classes may not be open).

Resolves:

Screenshot 2024-11-12 at 4 45 07 PM Screenshot 2024-11-12 at 4 45 25 PM

Because the APIs return the AuthCredential superclass, AuthCredential needs to be marked unchecked Sendable. All of it's subclasses need to then be marked unchecked Sendable or a warning will occur that the sendability of the superclass means the subclass should also explicitly mark itself as unchecked Sendable.

I added some API improvement ideas to the Firebase 12 gdoc.

no-changelog

ncooke3 commented 3 days ago

These classes have no mutable state so they should meet the semantic requirements of Sendable. In Firebase 12, we should refactor to checked Sendable. We can't currently do so without a breaking change because of the fact that the classes are open (Sendable classes may not be open).

~Only AuthCredential and OAuthCredential, PhoneAuthCredential are open.~ Ah, all are subclasses of an open class!

LGTM.

Yes, I think one approach to consider in the future is making them structs that conform to a common protocol (that conforms to Sendable). And for ObjC compat, make an ObjC only wrapper to preserve the current API.