-
Notifications
You must be signed in to change notification settings - Fork 794
Make transition actions async-first #452
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
Make transition actions async-first #452
Conversation
Further considerations: add Microsoft.Bcl.Async package for support below * `Task.GetAwaiter().GetResult()` is not supported before .NET Framework 4.5 * `Task.FromException(Exception)` is not supported before .NET Framework 4.6
We need TransitionConfiguration for InternalTransitionAsyncIf, PermitDynamicAsyncIf, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use EventCallback is not enought good solution. library can used in simple console host application that does not have this dependency
HenningNT
commented
Jun 20, 2021
Firstly, I must say I appreciate your hard work!
I'll take a look at this when I have time, as it changes quite a lot of files.
leeoades
commented
Apr 29, 2023
I know this was done a while ago but I like the idea of it a lot.
(I checked out the PR and obviously, the master branch has moved on since then so I wasn't trying to rebase it.)
I agree with the sentiment completely. It would remove all of the sync and async duplication.
My initial thoughts were that instead of needing EvenCallback, we could simply wrap all sync actions like so:
(plus the argument versions obviously)
public static class ActionExtensions
{
public static Task ToAsync(this Action action)
{
Task SyncWrapper(Action sync)
{
sync();
return Task.CompletedTask;
}
return SyncWrapper(action);
}
}
and then you would treat everything as async internally.
I believe the only thing to deal with then is someone calling Fire() instead of Fire.Async but I'm not sure a consumer would define async delegates but then call the sync version of Fire. But I think this can just use a Task .Wait().
The complication I hadn't considered was how EvenCallback deals with old skool Tasks and returns TaskResult. I need to get my head around supporting the older versions of .net pre the async await days.
I would be happy to give this another go though.
Uh oh!
There was an error while loading. Please reload this page.
This PR removes many duplicate codes that support both synchronous and asynchronous transition actions. It is done by introducing
EventCallback, part of ASP.NET Core, prioritizing asynchronous code, and making synchronous code proxy it. As a result, most public APIs are acting as-is, but some are changed to support more async way.Added
FireAsync()can execute sync transition actions, such asOnEntryChanged
OnTransitionedEventinvoke actions in order, regardless of async or syncOn*Asyncawait transition actions, regardless ofFireAsync()orFire()For synchronous non-blocking transition actions, use
On*and executeTask.Run()in an action.Fixed
OnEntryFromAsyncon generating graphs