Proposal: remove (or at least deprecate) all ReactiveCommand.CreateFromTask overloads that work with CancellationToken

[kentcb]

ReactiveCommand defines a number of CreateFromTask overloads that have funcs accepting a CancellationToken. I posit that these overloads are entirely useless. A full discussion on why this is so is included below.

Part of the impetus for this proposal is gathering feedback from community members who do use these overloads and, if so, how they are using them successfully.

Ideally, I would like to just remove these overloads. Yes, it's a breaking change, but given my statement above, no one should be using these overloads anyway, so we'd in theory be breaking nobody. At the very least, I want to deprecate these APIs and remove them in a later major revision (probably v8). Doing so would lead to a simpler, less confusing API. It would no longer mislead people into thinking these overloads are useful for cancellable TPL-based commands because they're not.

Below is an excerpt from some literature I'm writing. The context is a discussion on cancelling reactive commands.

CreateFromTask with CancellationToken is useless

If, on the other hand, you choose (or are forced) to used TPL for modelling command execution, things get a little trickier. In such cases, you should first be sure to accept a CancellationToken in your command’s execution logic:

private async Task<bool?> LoginAsync(CancellationToken ct)
{
    var result = this.Password == "Mr. Goodbytes";
    await Task.Delay(TimeSpan.FromSeconds(1.5),  ct);
    return result;
}

Secondly, be sure to use it! Notice that we’re passing the CancellationToken into Task.Delay. This will ensure that we bail as early as possible once the token indicates it has been cancelled (we’ll see how that happens shortly). If the API you’re consuming does not support cancellation, you will have to find some other recourse. Perhaps by submitting a pull request or switching to an alternative library. Failing that, you can always simulate cancellation by kicking off the task and periodically checking whether cancellation has been requested, bailing and ignoring the result if it has.

Finally – and this is where it gets less intuitive – it’s easiest to forgo CreateFromTask and instead use CreateFromObservable:

this.loginCommand = ReactiveCommand.CreateFromObservable(
    ()  =>
    Observable
        .StartAsync(this.LoginAsync)
        .TakeUntil(this.cancelCommand));

To convert our Task to an observable, we simply call Observable.StartAsync. There are overloads of StartAsync that work with both Task and Task<T>, with and without CancellationToken support. Since our LoginAsync method accepts a CancellationToken, we will receive a CancellationToken that is cancelled when the subscription to the observable is disposed. This makes it much easier to support cancellation of the command because we simply use the same TakeUntil trick as discussed above.

You may wonder why it’s better to use CreateFromObservable. You may look at the overloads for CreateFromTask and see that there are several that do use the CancellationToken type. If you try it out, you’ll see that you can do something like this:

this.loginCommand = ReactiveCommand.CreateFromTask(
    ct  => this.LoginAsync(ct),
    canLogin);

For clarity, I’ve made the ct parameter explicit here, but you could of course write this as:

this.loginCommand = ReactiveCommand.CreateFromTask(
    this.LoginAsync,
    canLogin);

Either way, this will cause ReactiveUI to perform the Observable.StartAsync call on our behalf, and our LoginAsync method still receives an appropriate CancellationToken. The problem is, of course, how do we actually cancel the command execution? We no longer have the ability to cancel based on a signal from our cancelCommand because we never get a say in the pipeline representing our command’s execution. We are only left with messy, far more onerous options.

The first thing we might try is maintaining our own CancellationTokenSource, creating a new one with each execution of loginCommand. We would then need to cancel that token manually when our cancelCommand executes. This results in extra state we need to manage and coordinate, and that state compounds for each cancellable command we create.

We might also consider the fact that ReactiveUI is calling Observable.StartAsync on our behalf. This means that the subscription to the command’s execution could be disposed to cancel the execution. However, we don’t normally create or manage that subscription ourselves. Usually, we’d use bindings so that all the peripheral functionality the accompanies commands – setting the target control’s enabled state, for example – just works. If we forgo binding and attempt to manage the subscription ourselves, we’ll quickly discover that this too leads to messy, convoluted code.

You could argue, of course, that this is all a design deficiency in ReactiveUI and that it should more readily support cancellation for TPL-based asynchrony scenarios. But if ReactiveUI attempted to do that, it would wind up in much the same dilemma as we did. Scattered and difficult to understand state would arise, and the API would become more complicated.

[escamoteur]

So how do you cancel running commands? I would love having them removed as they led to quite some frustration yesterday.

But if my Page Disposes it commands bindings, would this not automatically end execution?

[kentcb]

@escamoteur: the way to cancel TPL-based commands is discussed in the excerpt above.

[escamoteur]

Perhaps i just didn't understand it fully :-) Does this apply also for async Methods?

[escamoteur]

However, we don’t normally create or manage that subscription ourselves. Usually, we’d use bindings so that all the peripheral functionality the accompanies commands – setting the target control’s enabled state, for example – just works.

I don't fully agree with this, If you follow the WhenActivated Pattern with CompundDisposable you take care of the Subscriptions or not? But actuall this just supports your request to remove the overloads

[Hinidu]

We don't use these overloads but maybe it would be better to extend them with a cancellation argument (maybe of type IObservable<Unit>) than entirely remove them? It looks quite boring and hard to remember to always write code like this:

this.loginCommand = ReactiveCommand.CreateFromObservable(
    ()  =>
    Observable
        .StartAsync(this.LoginAsync)
        .TakeUntil(this.cancelCommand));

It looks like this implementation can be hidden in a library function looking something like that:

return ReactiveCommand.CreateFromObservable(
    ()  =>
    Observable
        .StartAsync(execute)
        .TakeUntil(cancel));

[kentcb]

@Hinidu yep, certainly an option worth considering.

[VistianOpenSource]

At one level, i'm not bothered whether these CreateFromTasks with CancellationTokens get binned off or not. I've used the framework for quite a few years, I'm very comfortable with observables rather than tasks.

However, and perhaps a little off topic, I am concerned however that another barrier is potentially being put up for those coming from the Task/async world. I'm sure if you did a poll I'm sure the vast majority of C# developers would say they are familiar with tasks & async, Reactive Extensions i'd hazard a guess would come a somewhat distant second.

Now you don't have to be a Reactive Extensions guru to get started with ReactiveUI, but to really gain benefit from the framework one really does need to get well embedded in the Reactive Extensions world.

As I said, its not directly related to your initial question, but I do think that when the roadmap is considered for ReactiveUI (and how to onboard people to the framework) then dealing with the potential lack of reactive extensions experience is imho one that needs serious consideration.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this because it helps reduce the workload of our extremely busy maintainers. If you want this issue progressed faster please start talks with a maintainer with how you can help out. There are so many ways to contribute to open-source and the most valued are often not code related. We are always looking for more help and look after those who stick around. Say howdy to one of the maintainers or browse https://reactiveui.net/contribute/ for ideas on where to start your journey.

[WorldMaker]

Isn't this just asking to add an optional IObservable<Unit> cancelOn = null parameter to CreateFromTask with CancellationToken to optionally add that TakeWhile operator? CancellationToken overloads are still useful in cases of command observable disposal even without a "cancelOn" trigger to force cancellation of specific tasks.

[ChrisPulman]

We are trying to come up with a resolve for this but the root cause is in the System.Reactive code https://github.com/dotnet/reactive/issues/1256 This issue has been outstanding for quite some time.

[ChrisPulman]

This should now be resolved, please raise a new issue with any new findings, many thanks

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.