Store: RFC: Could dispatch return a Promise and how do we handle canceled actions?

[markwhitfeld]

Request for comment:

What do you think of simplifying the dispatch method to return a Promise as opposed to an Observable?
Promises are more familiar in general and this case actually suits very well.
Promises are good for the scenario where there is a single return value and and there is a guarantee that it would return (with a result or error).
The async /await syntax would make this code very clear and expressive.

Following on from this, I think that what we decide around cancellation would help to guide this decision...
I have been wrestling with what a canceled action means to the end user.
The action could have been canceled by a subsequent action (ie. cancelUncompleted: true) or potentially a debounce, throttle, or skip (Action Pipes to come).
Which of the dispatch method observable callbacks would the user expect to be called (next, complete, error)?

1. If we view a canceled action to be an error path then the user may expect an error (Not sure I agree with this).
2. If we view a canceled action to be a special case of completion then we might expect just the complete to be called (How the current code works).
3. If we view a canceled action to be just another form of completion then the next and complete would be called.

Following on from this, what does it mean if the action is processed successfully by one handler and canceled on another?
Similarly, if an action is not handled by anything then does that mean anything special about it?
(Maybe there should be a console error for this case because I think it would usually be a mistake at dev time)

I'm favoring option 3 and the conversion to return a promise from the dispatch method to keep things simple.
I think the user dispatching the method doesn't need to know anything about cancellation or unhandled actions. It just makes things complicated.

Please chip in with your opinion. _The more opinions we get will help to create a more user friendly API._

[markwhitfeld]

_Just adding a forth option to the canceled action options..._

1. If we view a canceled action to be a noop then we don't have to make any response.
   This would simplify things internally because then we don't need to detect which actions are canceled but it breaks the paradigm of both the promise (expected to return something at least) and the observable (if we never complete then what happens to the subscriber?).
   I don't think that this is a reasonable option. It is too broken.

[jaripekkala]

+1 for returning a Promise from the dispatch method

[amcdnl]

What do you think of simplifying the dispatch method to return a Promise as opposed to an Observable?

No. I do agree they are more expressive BUT Angular uses Observables everywhere in its code. One of the main goals of this library is to create a state mgmt library that is designed for Angular (quirks and all). Also, since dispatch doesn't return anything, it gives you a good opportunity to chain selects to select your output, for instance:

store.dispatch(new Foo()).pipe(map(state.select(Foo.bar)).subscribe()

Since it returns a observable, you can all toPromise it if you still want this flow.

Which of the dispatch method observable callbacks would the user expect to be called (next, complete, error)?

I would expect complete to be called.

Following on from this, what does it mean if the action is processed successfully by one handler and canceled on another?

Good point. The same would apply if one is success and the other is error. In RX, if I were to have a observable with multiple observers in it and one fails it would fail them all. I think for consistency we should apply the same logic. Think about the case where I'm saving X, in saving X it calls 2 different actions, one to save the model to the DB and another to dispatch a websocket event. If they don't complete its not a successful action, something happened and i need to do take the error approach. Similarly if one was to cancel and another complete, it would fall into the complete case.

Similarly, if an action is not handled by anything then does that mean anything special about it?

Not all actions have to be handled by state so it should just automatically next/complete then.

[deebloo]

I agree with @amcdnl. keeping the observables provides us more flexibility. (admitting that provides its own set of challenges)

[markwhitfeld]

Thanks for your comments!
@amcdnl @deebloo I agree that the observable does give you more options for advanced cases. I think we should maybe add a comment in the documentation (and method summary) that you can use AsPromise() to use a simplified approach.

@amcdnl Just to be sure that I understand you correctly please could you answer the 2 questions below...

It looks like you are agreeing with option 2:

1. If we view a canceled action to be a special case of completion then we might expect just the complete to be called (How the current code works).

So, to be explicit, for cancelled actions complete would be called and not next or error?

And then, are you saying that for unhandled actions that both next and complete would be called?

[amcdnl]

So, to be explicit, for cancelled actions complete would be called and not next or error?

Yes

And then, are you saying that for unhandled actions that both next and complete would be called?

Yes

[splincode]

https://github.com/ngxs/store/issues/827