Angularfire: Feature Request: Wrap AngularFirestoreDocument API promise methods in Observables

[jdjuan]

Feature Request

1. Context

AngularFirestoreDocument has different promise-like methods: update, set and delete.
All of them return a Promise<void>.

db.collection<User>(Collections.user).doc(id).update({data}) // Promise<void>

AngularFire is great because it allows me to work with Observables under-the-hood and pipe different operators when fetching information, yet when it comes to updating, setting of deleting I need to wrap them inside fromPromise.

fromPromise(db.collection<User>(Collections.user).doc(id).update({data})) // Observable<void>

2. The Problem

Up to this point, that's ok, but it turns out that Promises, unlike Observables (like valueChanges()), fire right away without the need of subscribing (even inside of fromPromise).

db.collection<User>(Collections.user).valueChanges() // Cold Observable<User>
fromPromise(db.collection<User>(Collections.user).doc(id).update({data})) // Hot Observable<void>

So I believe we can make developers's life easier (and avoid race conditions bugs) if we standardize the AngularFirestoreDocument API into (Cold) Observables. This way, you don't have to pay extra attention if you're are dealing with promises or observables, hot or cold.

3. The Solution

Wrapping the API return values like this:

update(data: Partial<T>): Promise<void> {
    return defer(() => this.ref.update(data));
}

[hiepxanh]

@jamesdaniels should we make that update sir? What your opinion?
I don't know how it will impact. But i believe, If I want update, or delete, I will call action, manually ( it cold) and then my service should make an update, or delete immediately but not have to subscribe or doing anything more.

for example: an delete action. I will set an button, that delete contact. Only and Only I click that red delete button, and once it click, I need it delete now, so why we need turn it become cold?

So I believe that action like update or delete, which should be hot observables, should just be an promise.

What is your opinion? 😸
@jdjuan

[Supamiu]

I'd love to see this too but I'm wondering how breaking this change would be, let me explain:

At the moment, if I want to delete an item from the database, I'm calling db.collection<User>(Collections.user).doc(id).delete() for instance, and that's all.

If we change Promises for cold Observables, all the calls following the approach above will no longer work, as the operation will be cold, it'll need a .subscribe() appended, which would be a breaking change for a lot of applications using these calls.

That being said, being able to create cold operations would be really nice, as it allows to fully handle race conditions and gives more control over the general data persistence flow.

So maybe it would be like another method (prepareDelete?), just a random idea but maybe it would be a better approach to avoid breaking applications that are using the current Promise version.

[codediodeio]

I like this idea also, but only as an addition alongside Promises like @Supamiu mentioned. It would be a very breaking change and losing async/await might make some people sad (me).

It gets weird because Firebase performs optimistic updates, so your UI will update before an Observable emits. And if you unsubscribe before it emits your UI will still update, even though you canceled it.

In any case, I wouldn't mind having choices

// promise 
await ref.update(...);
console.log('done');

// rx
ref.update$(...).pipe(tap( () => console.log('done')).subscribe();

[jamesdaniels]

giong to close for now since I don't foresee any major changes in our APIs before we move to JS SDK vNext