Allow converters to return Promises

[justinfagnani]

Is your feature request related to a problem? Please describe.

I have some data that needs to be processed with async APIs when read or written to the database. I'd like to put the processing in a converter to ensure it's done consistently, but I can't because converters can't be async.

In this case the processing is compression/decompression. Without async converters I'm forced to choose between doing the processing at every read and write or tying up the main thread during compression.

Describe the solution you'd like

Converters should have an interface that allows for async methods:

  export interface FirestoreDataConverter<T> {
    toFirestore(modelObject: WithFieldValue<T>): DocumentData | Promise<DocumentData>;

    toFirestore(
      modelObject: PartialWithFieldValue<T>,
      options: SetOptions
    ): DocumentData | Promise<DocumentData>;

    fromFirestore(snapshot: QueryDocumentSnapshot): T | Promise<T>;
  }

We would need a new async method for reading data from snapshots and the existing .data() method would have to throw if an async converter is used:

    // Throws if the converter returns a Promise
    data(): T | undefined;
    dataAsync(): T | Promise<T> | undefined;

DocumentReference write methods set() and update() would need to support Promise fields, but they are already async.

Transaction write methods aren't async, but are obviously performing async work without blocking already.

Describe alternatives you've considered

Not using FirestoreDataConverters.

[wu-hui]

I will bring this up to my team for discussion. Will update here.

[wu-hui]

Hi @justinfagnani

After discussing with the team, we feel while this request has some merit, we are hesitant to add more API surface(dataAsync) so we could support this one feature. There might be a way to do it without adding dataAsync, but we did not have time to explore.

I'd put this in our backlog, and we might revisit this in the future. In the meantime, I believe you have reasonable workaround to get what you want.

Also, personally, I feel your use case is no longer standard deserialization, and it probably makes some sense that you will do something different to make it clear.

Thanks

[justinfagnani]

Sounds reasonable, thanks!

Also, personally, I feel your use case is no longer standard deserialization, and it probably makes some sense that you will do something different to make it clear.

I would push back on this a little bit. The only thing "non-standard" in terms of the APIs involved is that my deserialization is much better if async. Lots of other operations could be async and the JS ecosystem is moving to more and more prevasive async operations. Since the db calls themselves are async already, it seems pretty natural to fold in conversion into the existing async calls.