DECISIONS.md

Decisions

This file documents some of the decisions we made when developing Combine support for Firebase.

Module structure

Discussion

The general idea is to keep all Combine-related code in a separate module (FirebaseCombineSwift, to match the naming scheme used for FirebaseFirestoreSwift and FirebaseStorage).

By using the #if canImport(moduleName) directive, we can make sure to only enable the publishers for a module that developers have imported into a build target.

Implementing Publishers

Custom Publishers vs. wrapping in Futures / using PassthroughSubject

Instead of implementing custom publishers, which Apple discourages developers from doing, we make use of PassthroughSubject (for publishers that emit a stream of events), and Future for one-shot calls that produce a single value.

Using capture lists

After discussing internally, we came to the conclusion that the outer closure in the following piece of code is non-escaping, hence there is no benefit to weakly capture self. As the inner closure does't refer to self, the reference does not outlive the current call stack.

It is thus safe to not use [weak self] in this instance.

extension Auth {
    public func createUser(withEmail email: String,
                           password: String) -> Future<AuthDataResult, Error> {
      Future<AuthDataResult, Error> { /* [weak self]  <-- not required */ promise in
        self?.createUser(withEmail: email, password: password) { authDataResult, error in
          if let error = error {
            promise(.failure(error))
          } else if let authDataResult = authDataResult {
            promise(.success(authDataResult))
          }
        }
      }
    }
}

Method naming

Discussion

• Methods that might send a stream of events over time will receive a Publisher suffix, in line with Apple's own APIs. Any add prefix will be removed. This helps to clarify that the user is not adding something that they will have to remove later on (as is required in most of Firebase's existing APIs). Instead, the result of the publisher needs to be handled just like any other publisher (i.e. be kept in a set of Cancellables).

  Examples:

  • addStateDidChangeListener -> authStateDidChangePublisher
  • addSnapshotListener -> snapshotPublisher

• Methods that return a result once will not receive a suffix. This effectively means that these methods are overloads to their existing counterparts that take a closure. To silence any Result of call to xzy is unused warnings, these methods need to be prefixed with @discardableresult. This shouldn't be a problem, since the Future that is created inside those functions is called immediately and will be disposed of by the runtime upon returning from the inner closure.

  Examples:

  • signIn -> signIn
  • createUser -> createUser

Options considered

Using the same method and parameter names for one-shot asynchronous methods results in both methods to be shown in close proximity when invoking code completion

To achieve the same for methods that return a stream of events, we'd have to name those addXzyListener. This would be in contrast to Apple's naming scheme (e.g. dataTask(with:completionHandler) -> dataTaskPublisher(for:)