Feature ReactiveCommand.CreateRunInBackground

[ChrisPulman]

What kind of change does this PR introduce?

Feature for ReactiveCommand

What is the current behaviour?

ReactiveCommand.Create runs Synchronously
ReactiveCommand.CreateFromTask runs Asynchronously
ReactiveCommand.CreateFromObservable runs Asynchronously

What is the new behaviour?

To give the option for all Creation options to operate in the same way a new method CreateRunInBackground has been added to the ReactiveCommand creation options making:
ReactiveCommand.Create runs Synchronously
ReactiveCommand.CreateRunInBackground runs a Method Asynchronously
ReactiveCommand.CreateFromTask runs a Task Asynchronously
ReactiveCommand.CreateFromObservable runs a Observable Asynchronously

What might this PR break?

None expected as existing functionality is default

Please check if the PR fulfils these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:

[codecov]

Codecov Report

Patch coverage: 97.69% and project coverage change: +0.30 🎉

Comparison is base (42929d4) 63.60% compared to head (0182a79) 63.91%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3501      +/-   ##
==========================================
+ Coverage   63.60%   63.91%   +0.30%     
==========================================
  Files         157      157              
  Lines        5768     5783      +15     
==========================================
+ Hits         3669     3696      +27     
+ Misses       2099     2087      -12     

Impacted Files	Coverage Δ
...eactiveUI/ReactiveCommand/ReactiveCommandMixins.cs	96.96% <88.88%> (+0.04%)	⬆️
src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs	84.11% <99.08%> (+1.28%)	⬆️
...ctiveUI/ReactiveCommand/CombinedReactiveCommand.cs	84.31% <100.00%> (+1.29%)	⬆️
.../ReactiveUI/ReactiveCommand/ReactiveCommandBase.cs	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[glennawatson]

Can we change the new flag to be not a optional parameter.

Optional parameter are a breaking API and can break with certain tools, while a method overload is not.

[anaisbetts]

I have a few questions about this PR, I think we could maybe simplify this implementation quite a bit and make its intent a bit more clear

• ReactiveCommand.CreateAsync - this is a really ambiguous name, especially in the context of the CreateXXX methods - I think a better name would be something like ReactiveCommand.CreateRunInBackground, where its intended use is more explicit.

Also, I think we can really simplify its implementation, as just a simple proxy to CreateFromObservable, rather than making all of these internal changes:

public static ReactiveUI.ReactiveCommand<TParam, TResult> CreateRunInBackground<TParam, TResult>(
        Func<TParam, TResult> execute, 
        IObservable<bool>? canExecute = null,
        // Rather than "runAsync", always provide a way to control concurrency via an IScheduler
        IScheduler? backgroundScheduler? = null,
        IScheduler? outputScheduler = null) {

    return ReactiveCommand.CreateFromObservable(
        Observable.Start(execute, backgroundScheduler ?? RxApp.TaskPoolScheduler), canExecute, outputScheduler);
}

Thoughts?

[ChrisPulman]

I have a few questions about this PR, I think we could maybe simplify this implementation quite a bit and make its intent a bit more clear

• ReactiveCommand.CreateAsync - this is a really ambiguous name, especially in the context of the CreateXXX methods - I think a better name would be something like ReactiveCommand.CreateRunInBackground, where its intended use is more explicit.

Also, I think we can really simplify its implementation, as just a simple proxy to CreateFromObservable, rather than making all of these internal changes:

public static ReactiveUI.ReactiveCommand<TParam, TResult> CreateRunInBackground<TParam, TResult>(
        Func<TParam, TResult> execute, 
        IObservable<bool>? canExecute = null,
        // Rather than "runAsync", always provide a way to control concurrency via an IScheduler
        IScheduler? backgroundScheduler? = null,
        IScheduler? outputScheduler = null) {

    return ReactiveCommand.CreateFromObservable(
        Observable.Start(execute, backgroundScheduler ?? RxApp.TaskPoolScheduler), canExecute, outputScheduler);
}

Thoughts?

Thanks Ani, I will update this evening to realign to these thoughts and test to ensure the result is as desired.

[github-actions]

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