-
Notifications
You must be signed in to change notification settings - Fork 768
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
base: dev
Are you sure you want to change the base?
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...
{ | ||
readonly TState _state; | ||
private readonly EventCallback _callback; |
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
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. |
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:
and then you would treat everything as async internally. 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. |
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 asOnEntry
Changed
OnTransitionedEvent
invoke actions in order, regardless of async or syncOn*Async
await transition actions, regardless ofFireAsync()
orFire()
For synchronous non-blocking transition actions, use
On*
and executeTask.Run()
in an action.Fixed
OnEntryFromAsync
on generating graphs