Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding AsyncDelegateCommand #2907

Merged
merged 3 commits into from
Aug 4, 2023
Merged

Adding AsyncDelegateCommand #2907

merged 3 commits into from
Aug 4, 2023

Conversation

dansiegel
Copy link
Member

Description of Change

Adding support for an AsyncDelegateCommand. By default this will prevent simultaneous execution of the Task. This behavior can be opted out of by calling the EnableParallelExecution() method on the AsyncDelegateCommand. Note that we will suppress the TaskCanceledException to prevent this from being raised when a CancellationToken cancels the Task.

// Parameterless Callback
var cmd1 = new AsyncDelegateCommand(Command1);
Task Command1() => Task.Delay(500);

// Callback with a CancellationToken
var cmd2 = new AsyncDelegateCommand(Command2);
Task Command2(CancellationToken token) => Task.Delay(500, token);

// Callback with our Generic model type
var cmd3 = new AsyncDelegateCommand<MyModel>(Command3);
Task Command3(MyModel model) => Task.Delay(500);

// Callback with our Generic model type and CancellationToken
var cmd4 = new AsyncDelegateCommand<MyModel>(Command4);
Task Command4(MyModel model, CancellationToken token) => Task.Delay(500, token);

Since normal usage of the Command will involve the Application Framework invoking ICommand.Execute(object), an additional method has been provided to allow developers to provide a default CancellationToken factory. As an example we could provide a delegate that ensures we get a CancellationToken that will automatically Cancel after 500 milliseconds.

var cmd = new AsyncDelegateCommand(OnExecute)
    .CancellationTokenSourceFactory(() => {
        var cts = new CancellationTokenSource()
        cts.CancelAfter(500);
        return cts.Token;
    });

Bugs Fixed

@dansiegel dansiegel merged commit bf0966a into master Aug 4, 2023
5 checks passed
@dansiegel dansiegel deleted the dev/ds/asyncdelcommand branch August 4, 2023 21:47
@bdovaz
Copy link
Contributor

bdovaz commented Sep 1, 2023

@brianlagunas @dansiegel why has this improvement not been included in 8.x? I think it would be very beneficial for everyone to have this improvement right now without having to wait for 9.x which I don't know when it will be released.

Thank you!

@dansiegel
Copy link
Member Author

@bdovaz that's not how new features work... new features go into new releases not old ones

@bdovaz
Copy link
Contributor

bdovaz commented Sep 1, 2023

@dansiegel I disagree because according to SemVer that feature could go directly into 8.x because it is not a breaking change.

Think that 9.x may take a year (to say a fact) to release a stable version and that feature is something very useful.

@dansiegel
Copy link
Member Author

Your argument hasn't been thought through. So you're saying you'd be happy to wait for an 8.X version but not a 9.0 release. New features go into the next release. Whether that was going to be 8.2 or 9.0 is irrelevant. We have bumped to 9.0 for a variety of reasons that have little to do with just AsyncDelegateCommand. The next release that will have this would come out at the same time regardless of version number.

@bdovaz
Copy link
Contributor

bdovaz commented Sep 1, 2023

Sorry, so I misunderstood the term "next release".

I was thinking you were referring to 9.x. That's why my concern, because I understood that 8.x and 9.x were going to be released in parallel but that 9.x was quite far from a stable version.

@dansiegel
Copy link
Member Author

9.0 is planned to be stable with net8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] AsyncDelegateCommand
3 participants