[Firestore] Add async counterpart for the `addDocument` method

[MojtabaHs]

The async version of the addDocument function exists only for Dictionary input, so I have added a codable version as well using the original existing methods.

• The function signature is from the original; I just converted the completion parameter to async.
• The documentation is from the original; I only added the Throws line from another existing function.
• The implementation is from another existing function, but it uses the original addDocument which takes an encodable.

[paulb777]

Thanks @MojtabaHs! I'd rather see this added to Firestore/Swift/Source/Codable/CollectionReference+WriteEncodable.swift since that file is already Swift and it would be closer to the non-async implementation.

@ncooke3 @wu-hui Do you know if it was an oversight that this is missing or some other reason?

[paulb777]

@MojtabaHs Thanks for the PR. Given the open questions we're seeing, we want to a deeper review of the Firestore APIs and make sure we're being consistent with how we handle async and throwing. It may take us a while longer to move forward here.

[MojtabaHs]

@paulb777 Let me know if there is anything I can do to help this progress.

[MojtabaHs]

Any updates on this? There are other opportunities to introduce async counterparts to modernize the API, but I’m concerned that this decision could affect those efforts as well. I believe it’s important to address this first.

[ncooke3]

Hi @MojtabaHs, no updates as of yet. This is something actively being looked into. I'll update when the direction is more clear.

[MojtabaHs]

It's been a while, how can I assist you in moving forward with this progress?

[MojtabaHs]

Rebased on the current main branch.

Any updates? It’s been a while, and I’m concerned about losing context.

[paulb777]

Sorry for the delay here. We're focusing on Swift Concurrency improvements in other Firebase products before getting back to Firestore. It may be a few more months.

[MojtabaHs]

Rebased on the current main branch.

[mortenbekditlevsen]

As the poster of #9157 , I strongly discourage adding this API (for the same reason as doing it in RTDB).

Having an async function that never returns (when you're offline) could be highly unexpected. How a never returning async call would propagate to also let all callers never return would seem like a really hard issue to debug.

Using a callback closure suggests that this is perhaps a power user API that should be considered carefully...

@MojtabaHs , you mention the possibility of deprecating the existing async API that takes a Dictionary of data. I think that this is the better approach.

I assume that the existence of the async API is due to how Objective-C automatically imports to Swift, and it could be that this API has just not been vetted for adding attributes to explicitly not add an async version of the API.

I'd much rather see an entirely new API in the shape:

func addDocument(..) async throws -> SyncResult {}

enum SyncResult {
  case enqueued
  case synchronized
}

• or something to that effect - although I don't know if the underlying technology would support this.

[Sam-Spencer]

@mortenbekditlevsen I read through the discussion on the issue you referenced... someone there suggested returning an AsyncStream, which could emit events with additional details (e.g. to inform the caller that the client is offline and this will be tried later). That sort of implementation seems like it would provide both swift concurrency support and address the issues you're calling attention to here.

[mortenbekditlevsen]

@Sam-Spencer It sounds like that could be a really great way to model that API.

Perhaps there could also be an event representing the situation where your write was made redundant by a later write or a remove - all while you were offline.