Add Combine support to Firestore

[lorenzofiamingo]

I know Combine support for Firebase is not even discussed, so please feel free to ignore me. I make this pull request only in the remote case it could be useful to you.

Discussion

• Implementation based on this Result support implementation: Add Result argument type support to Firestore methods closures #6940

• Some info regarding my proposal (I have doubts on some points):

  • I added to every method the publisher suffix (Following the pattern used in URLSession method dataTask that becomes dataTaskPublisher.
  • The one-time requests are mapped with Future. I decided to leave the Future return type to make more evident the fact that after the emission of the first value the publisher receives a completion.
  • Original addDocument in CollectionReference method already has a return type. Instead of returning a tuple, I decided to pass the DocumentReference to the publisher's output, so the reference will be available only asynchronously. In my opinion, if you want to use a publisher for this task, is more useful to have a clean return type than handle it synchronously. This considering the fact that you can directly create a new reference and then set data. Maybe creating a second method with a tuple as a return type would be useful.

• Other approaches:

  • Creating custom publisher conforming to Publisher/Subject/ConnectablePublisher protocols. With this approach would be nice to group different methods in the same publisher using Combine backpressure. In this way, for example, depending on subscriber request demand, different methods could be called.
  • Grouping publishers in struct/enums. For example: myreference.fetcher.publisher(continously: true)

Testing

• Test missing

[wilhuff]

In general, Firestore (and Firebase more broadly) have tried to avoid baking in any strong preference for any specific UI or data processing frameworks to our base SDKs. We provide the building blocks you can use for using the API simply without requiring users to necessarily go learn additional, potentially complex frameworks. We also want to avoid giving preference to one framework over another at this layer. This is why in the JavaScript SDK, AngularFire and EmberFire are separate packages from the base SDK.

Combine is a bit of an interesting case because unlike the other examples, it's built into platform. I'm still inclined to exclude it because it adds additional methods that complicate the interface even if all you wanted was Codable support when you added FirebaseFirestoreSwift. I'm wary of giving preference to Combine over something like RxSwift, and I'm also wary of committing to a specific way of using Firestore with Combine. Some of the callouts you made in the PR description make me uneasy since there are cases where there's no obviously right way to do this.

It seems like this might be better off in a separate FirebaseFirestoreCombine package that lives in FirebaseExtended. @ryanwilson @peterfriese @morganchen12 thoughts?

[morganchen12]

+1, I lean toward making it a separate framework as well. If you want to not worry at all about your Combine extension being opinionated, you could host it in your own repository outside of Firebase entirely and then submit a pull request to add it to http://firebaseopensource.com/.

[lorenzofiamingo]

@wilhuff I think should be considered also the fact SwiftUI data flow is based on Combine.
@morganchen12 No problem with being opinionated, indeed that's what I wanted.

Since Combine principal feature is composability, the best would be most firebase components to adopt it, so the ideal, in my opinion, would be a separate package FirebaseCombine, that activate Combine methods, (if present) for each component installed.

[peterfriese]

Hi @lorenzofiamingo -

we've integrated the groundwork for Combine into combine-main, and have started implementing support for Firebase Auth.

Would you be open to updating this PR according to the decisions we laid out in this file?

As #6940 hasn't been merged yet, I would suggest mapping results manually (like here, for example).

Don't forget to add tests for any new code.

Thanks,
Peter

[lorenzofiamingo]

Hi @peterfriese!

I spied on your commits as they were published and they are really cool!
I very much agree with these decisions.

At the moment I'm in the exams period at university, but I'll do my best as soon as I got some spare time.
If you need to get this done quickly, feel free to modify or overwrite it (obviously).

[ryanwilson]

At the moment I'm in the exams period at university, but I'll do my best as soon as I got some spare time.
If you need to get this done quickly, feel free to modify or overwrite it (obviously).

Good luck on your exams @lorenzofiamingo! 😄 take your time, studying comes first 😉

[lorenzofiamingo]

Closed in favor of #7623