Windowscommunitytoolkit: [Feature] Basic MVVM primitives (.NET Standard)

Created on 9 Apr 2020  ยท  48Comments  ยท  Source: windows-toolkit/WindowsCommunityToolkit

Describe the problem this feature would solve


Follow up from the conversation with @michael-hawker on the UWP Community Discord server.

With the MvvmLight library not being supported anymore, and other existing libraries being much bigger in scope that what is often needed for simple apps, I was wondering whether we should add some basics MVVM primitives to the toolkit, possibly in a new Microsoft.Toolkit.Mvvm package so that users not in need of those wouldn't have to reference those too. We're also using this occasion to integrate with the Microsoft.Extensions.DependencyInjection package to provide dependency injection features, so that it'll be both easier for devs to interoperate between this package and existing code, as well as allowing us to offload that part of the code to an official library from Microsoft.

All this would come together in a new Microsoft.Toolkit.Mvvm package.

Preview package

The CI is now running for #3229 , so you should be able to just get the latest package from the CI build artifacts.

Key Tenants

  • Platform and Runtime Independent - .NET Standard 2.0 ๐Ÿš€ (UI Framework Agnostic)
  • Simple to pick-up and use - No strict requirements on Application structure or coding-paradigms (outside of 'MVVM'ness) i.e. flexible usage
  • ร€ la carte - Developer able to choose the components they wish to leverage
  • Reference Implementation - Lean and performant, provides compliments to interfaces and paradigms hinted at in the Base-Class Library, but without provided implementations.

Draft docs here ๐Ÿ“–

Feature set ๐Ÿงฐ

The idea for this package would be to have a series of common, self-contained, lightweight types:

  • Microsoft.Toolkit.Mvvm.ComponentModel

    • ObservableObject

    • ViewModelBase

  • Microsoft.Toolkit.Mvvm.DependencyInjection

    • Ioc

  • Microsoft.Toolkit.Mvvm.Input

    • RelayCommand

    • RelayCommand<T>

    • AsyncRelayCommand

    • AsyncRelayCommand<T>

    • IRelayCommand

    • IRelayCommand<in T>

    • IAsyncRelayCommand

    • IAsyncRelayCommand<in T>

  • Microsoft.Toolkit.Mvvm.Messaging

    • Messenger

    • IMessenger

  • Microsoft.Toolkit.Mvvm.Messaging.Messages

    • PropertyChangedMessage<T>

    • RequestMessage<T>

    • AsyncRequestMessage<T>

    • CollectionRequestMessage<T>

    • AsyncCollectionRequestMessage<T>

    • ValueChangedMessage<T>

These types alone are usually enough for many users building apps using the MVVM toolkit.
Having these built right into the toolkit would allow them to just reference the new package and bet all set, without having to go dig through other documentation and referencing a series of external packages just to gain access to these basic types. Furthermore, we could keep these smaller in scopes than other big libraries like autofac or Prism, which include a lot of features that are not necessarily useful for many (particularly non-enterprise) devs, while still taking a toll on performance and memory usage. This would also make the learning curve much less steep for developers.

As an example of the benefit of a more modern codebase, which is both more optimized and minimalistic, here's a benchmark showing the performance of the Messenger class from both MvvmLight and Calcium, and the one from the related PR (WCT stands for this new Mvvm package). Note that Calcium only supports a single channel mode:

image

Some summarized results for the Messenger class from #3229:

  • Single channel: ~40x faster than MvvmLight, and ~11% faster than Calcium. At the same time, it uses about 38 million times less memory than MvvmLight, specifically just 20 bytes vs. almost 700MB (!), and about 387,000x less memory than Calcium as well, specifically again just 20 bytes vs. about 7 MB.
  • Multiple channels: about 11x times faster than MvvmLight, and using almost 7 million times less memory, specifically just 416 bytes vs almost 3 GB (!).

The full benchmark code can be found here.

Describe the solution


As mentioned above, and as can be seen in the related PR #3229, this proposal is to add a new Microsoft.Toolkit.Mvvm package, integrating some features from MvvmLight, refactoring them when needed, in order to offer a self-contained alternative. The package _would not need a reference_ to any other Microsoft.Toolkit packages, it would be lightweight and focused on ease of use, performance and memory efficiency.

Since it targets .NET Standard 2.0, this means that it can be used anywhere from UWP apps, to Uno, Xamarin, Unity, ASP.NET, etc. Literally any framework supporting the .NET Standard 2.0 feature set. There are no platform specific dependencies at all. The whole package is entirely platform, runtime, and UI stack agnostic.

Additionally, it would integrate with the existing Microsoft.Extensions.DependencyInjection package, to offer a well known and tested DI platform, with the addition of an easy to use entry point in the toolkit, especially for new users. This would also remove a lot of code from the actual Mvvm package, as there would not be the need to write and maintain _yet another_ DI library or set of APIs.

As mentioned above, the package would target .NET Standard 2.0 to have as much reach as possible, and it wouldn't have any platform-specific dependencies.

Having a set of MVVM-enabled features built-in into the toolkit would align with the general philosophy of the toolkit to help developers getting started in common scenarios (such as with the MVVM pattern), and it would complement all the available APIs from the BCL that just lack a "reference" implementation (such as INotifyPropertyChanged or ICommand).

Furthermore, having a package like this being published by a Microsoft supported project directly would be a big selling points for users not wanting to add "3rd party" references to their projects (this point has been brought up before in the comments below as well by other users).

Describe alternatives you've considered


As mentioned above, common alternatives like autofac, Fody or Prism are much bigger in scope than what many devs typically need and introduce more overhead. MvvmLight on the other hand is also not supported anymore, other than some forks.

.NET Standard Completed feature feature request open discussion

Most helpful comment

Relaying some updates here from the UWP Discord server to keep everyone up to date.

Following an extensive conversation with @Aminator and some prototyping, I went ahead and did a number of changes in #3229, specifically related to the dependency injection area:

  • Completely nuked the old Ioc class/interface.
  • The package now has a dependency on Microsoft.Extensions.DependencyInjection so we can use those types directly (eg. IServiceProvider), which make the package better for interoperability, saves us work/test/maintenance and can also act as an easy to use "entry point" for users to become familiar with the APIs from that package. @azchohfi I think you'll be happy about this since you were the one that suggested that package from the start ๐Ÿ˜Š
  • With the better integration now, the ViewModelBase class now takes the general IServiceProvider interface in its secondary constructors, so users also have the ability to do DI with their own service providers coming from somewhere else.
  • On the topic of better integration, with this change we'd now have both the toolkit and the Windows Template Studio all using the same exact DI APIs, coming from an official 1st party Microsoft package (Microsoft.Extensions.DependencyInjection).
  • The Ioc class now acts as a super easy to use service configurator for those APIs.
  • Tested this already on UWP Debug/Release as well, works just fine.

Sample usage of how to configure services with the new Ioc class:

Ioc.Default.ConfigureServices(services =>
{
    services.AddSingleton<ILogger, Logger>();
    services.AddSingleton<IPrinter, PrinterService>();
    // ...
});

The Ioc class then exposes an IServiceProvider ServiceProvider property which is automatically used by ViewModel base if no other provider is specified, which means that users can just use that code to initialize services, and then every single ViewModel in their app will automatically have the resulting IServiceProvider instance being injected. They can then just use all the extensions available from the Microsoft.Extensions.DependencyInjection package to get service instances from that provider.

This solution has the advantage of being both easy to use, highly customizable, and using the same APIs from Microsoft.Extensions.DependencyInjection while still having the same behavior from the original Ioc class from MvvmLight. Plus we don't have any code to maintain at all in that area.


Other general changes from the PR (and a few more points for @skendrot I forgot to mention the other day). Regarding having basic MVVM building blocks in the toolkit, I realized the toolkit actually already does have a few of them. In particular Singleton<T> and NotifyTaskCompletion<TResult>:

  • The Singleton<T> is scheduled for removal in 7.0 already (see #3134), but that's just for performance reasons and not lack of usage. In fact, that class was basically a minimalistic inversion of control container. With the new Mvvm package, users relying on that class will be able to easily switch to the new Ioc class without losing any functionality (and gaining the additional features).
  • The NotifyTaskCompletion<TResult> class is being replaced by the SetAndNotifyOnCompletion method from ObservableObject, which exposes the same functionalities (and more) without requiring dedicated types at all, which also makes it more efficient and makes the code easier to read. Here is how the new method can be used in a small sample:
private Task<string> myTask = Task.FromResult<string>(null);

public Task<string> MyTask
{
    get => myTask;
    private set => SetAndNotifyOnCompletion(ref myTask, () => myTask, value);
}
<TextBlock>
    <Run Text="Task status:"/>
    <Run Text="{x:Bind ViewModel.MyTask.Status, Mode=OneWay}"/>
    <LineBreak/>
    <Run Text="Result:"/>
    <Run
        xmlns:ex="using:Microsoft.Toolkit.Extensions"
        Text="{x:Bind ex:TaskExtensions.ResultOrDefault(ViewModel.MyTask), Mode=OneWay}"/>
</TextBlock>

Which results in this with a function that waits two seconds and then returns "Hello world!":

NVXa3N8Wom

Same feature as before (but in a dedicated, specialized package now), and without the additional "proxy" object, so that the property you have in your models is actually of type Task<T> ๐ŸŽ‰

All 48 comments

Thanks for submitting a new feature request! I've automatically added a vote ๐Ÿ‘ reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

To add to this, I had ported @StephenCleary's MVVM Async helpers from his articles to UWP here. Though I didn't realize he had them on GitHub already here (though not sure if UWP enabled).

Also, Windows Template Studio has an MVVM Basic pattern with some basic helpers, this could be an opportunity to align those in the toolkit instead of part of their template engine. FYI @sibille.

@Sergio0694 I think the best place to start before we dive deeper in code, is to talk more to @lbugnion and @sibille and figure out the best path forward (even if community driven) for longer-term support of a light-weight MVVM framework.

That sounds like a great idea! Looking forward to hearing what they think too! ๐Ÿ˜„

As for the PR, I've mostly just opened it as a draft to have a reference, and because I wanted to experiment with that Ioc idea I had (which I'll definitely use at least in my apps either way given those benchmarks ๐Ÿš€). Wasn't planning to work more on it for the time being, as I completely agree with you on gathering more feedback first, and besides I think the fork that @jamesmcroft made of the MvvmLight already contains most of the basic building blocks we'd need, at least for a prototype.

Looking forward to seeing where this goes, glad I could at least spark a discussion on the topic! ๐Ÿ˜Š

@Sergio0694 how does your implementation compares to the ASP.Net default one? https://www.nuget.org/packages/Microsoft.Extensions.DependencyInjection/5.0.0-preview.2.20160.3
I'm not even sure it will work on UWP, but it should, as there is a .Net Standard 2.0 version of it.
This might help as well: https://github.com/danielpalme/IocPerformance

And are these numbers for .Net Native?

@azchohfi Those are numbers for .NET Core 3.1, since BenchmarkDotNet doesn't support UWP. I did setup a benchmark class to test things on UWP too a while back though, it's not perfectly accurate as BenchmarkDotNet (and it lacks the memory diagnoser) but it's reliable enough, I'll give it a go to see how it runs there! As for the ASP.NET one, not sure if it works on UWP either, but I can definitely at least add that to the .NET Core benchmarks and see how it goes, thank you for the suggestion! ๐Ÿ˜„

Will update the post as soon as I have more results.

EDIT: updated the post, the ASP.NET solution is faster than MvvmLight but it's still about 18x slower than the Ioc class in this PR. Of course, the ASP.NET version much like the other 2 libraries has more features, though I'll say that the Ioc class in this PR has the advantage of being much easier to setup, of offering static APIs so you don't necessarily need to pass instances of your container around, plus it's all self contained in a single file and with no dependencies. Different use cases, sure ๐Ÿ˜Š
I'll go ahead and test on UWP too!

EDIT 2: done, it's all in the first post! ๐Ÿ‘

@azchohfi Not sure whether GitHub sends notifications when mentioning through an edit to an existing comment, posting this just in case and also to add some more details ๐Ÿ˜„
Since you seemed interested in performance metrics, I run some more benchmarks across all the various implementations, both when retrieving singleton services as well as transient ones.
Here are the updated results (.NET Core 3.1):

image

The situation with singleton services is like before, whereas when retrieving transient ones, this Ioc implementation is about 60x faster than MvvmLight (thanks to compiled expressions) and using less than 1/11th the memory, and it remains over twice as fast as the ASP.NET solution. ๐Ÿš€

I recall seeing a Visual Studio survey for MVVM feedback on Twitter a few months back. It appeared there was possible work being done to add MVVM basics to the framework to help in this area. Just curious, is this the same work or is it possible there is duplicate work being investigated?

Hey @shaggygi - this is not the same work, it's more of an exploratory issue to put some ideas together and see how best to proceed. We're talking with @jamesmcroft who is maintaining a .NET Standard fork of MvvmLight, and @michael-hawker has contacted both Laurent, the original author of the MvvmLight library, and the folks working on the Windows Template Studio, to see if it could make sense to integrate some of the existing work into the toolkit, or to just link an external repository and update that one, etc.

The draft PR that is linked instead was just me experimenting with some ideas in the meantime ๐Ÿ˜„

@sergio0694 and @michael-hawker I think adding basic MVVM classes to the Toolkit makes a lot of sense.

Among the goals for the MVVMBasic implementation in Windows Template Studio was to provide a solution for people who:

  • can't or don't want to use a 3rd party MVVM Framework
  • want to build their own MVVMFramework

https://github.com/microsoft/WindowsTemplateStudio/blob/dev/docs/UWP/frameworks/mvvmbasic.md

Not sure if this is important to a lot of people, if it is, in WTS we could still offer people both options MVVMBasic (using the WCT) or MVVMBasic (generating the classes in the code).

Hi @sibille - thank you for chiming in!
I'm glad to hear this idea makes sense to you, that's great! ๐Ÿ˜Š

And yes, if the WTS then offered both the option to either reference the MVVM classes from the WCT directly, or having them autogenerated into a project, that'd be ideal, good idea! ๐Ÿ˜„
Maximum flexibility for users, and offering those who don't mind using an external library the ability to use a compact set of classes that are more up to date, and right from the WCT ecosystem.

I've added the new Messenger implementation (code here), which completely avoids the use of reflection and is able to have no memory allocations at all when sending messages (and it's also faster than the messenger from MvvmLight). Here's a benchmark of the two Messenger class (gist here), when sending a number of messages between multiple receivers, either through the main channel (no token), or through various separate channels (using a token to distinguish them):

image

You can see that not only is the new Messenger class much faster than the one from MvvmLight (especially when using the default channel), but it also literally does no memory allocations at all, whereas the MvvmLight type ends up allocating almost 500MB (!) in this benchmark ๐Ÿš€

P.S. the PR should be more or less be feature complete at this point, since the idea was just to have the basic types available. Will go ahead and add more unit tests, as they are just a few for now.

Adding "basic" mvvm tooling has been discussed numerous times, both publicly and privately.
Here are a few I found easily:

  • #556: Move commands from sample project to Toolkit
  • #831: INotifyPropertyChanged base class
  • #943: Add DelegateCommand/RelayCommand to the Toolkit
  • #2444: Rx Mvvm Framework
  • #2893: Do we need a Messenger system or event bus for the pub-sub pattern?

There are a lot of frameworks already out there.
Just to name a few:

  • MVVM Light
  • Prism
  • Caliburn Micro
  • MVVM Cross

MVVM Light is just one implementation. If it seems that MVVM Light needs a maintainer or work to make it better or "more light" wouldn't it be best to put work into that project? It's already established, it's well known, and it's it's heavily used (as a MVVM framework. Not saying this toolkit isn't used).

MVVM Frameworks always start out as something small. Then more and more features are added making them larger and larger. Creating a framework within the toolkit would just explode to being more and more.

My suggestion is that we should maintain the stance that there are already frameworks out there and that's not what this toolkit is meant for.

About the IoC part: I would really like it if there would be a bit more standardization on dependency injection for UWP, like Asp.Net Core has. Benefits are a clearer approach for everybody (helps to find unified documentation) and aligned efforts rather than scattered efforts. In that sense, personally, I would rather see standardization on Microsoft.Extensions.DependencyInjection. I find the performance you achieved impressive though!

On the MVVM part: I would really like to see a MVVM approach for UWP supported by Microsoft and/or the WCT. The issue with MVVM for UWP currently is that as a developer I do not feel confident at all taking a dependency on any of the established 3rd party frameworks. That is because e.g. they seem to have dropped support for UWP or seem to have stopped being maintained. That is why I am really looking for a clear, reliable, supported way of doing things for UWP. If moving MVVM into the WCT is the way to do that, please go forward.

I know about this XKCD comic about standards. However, I think we got into this situation because, for UWP projects, there was never a clear drive from Microsoft on these two points (MVVM & IoC/DI). Supporting MVVM frameworks for a longer time takes a large effort which I understand, but developers taking a dependency on a MVVM framework do that because they develop a more complex app that needs usually needs support for a longer time. Then it feels scary to take a dependency on a 3rd party MVVM framework that might lose support.

Hey @hansmbakker - glad you liked the performance improvements in the Ioc class! I'll try to go through all the various points that you and @skendrot raised one at a time.

Regarding the various closed PRs you linked, I feel like some context is important, specifically, the timing. Most of those issues are pretty old (some from 2017), and a lot of things have changed. If you mention the overlap in different frameworks, I'd argue that the Windows Community Toolkit and MvvmLight actually _already_ overlap in quite a few places - for instance, MvvvmLight has a whole helper class that's UWP-specific that handles dispatching to the UI thread, which is exactly the same functionality that the toolkit offers through the DispatcherHelper class.

Having a built-in set of MVVMs types boils down to a few key points:

  • Some of the frameworks you mentioned (eg. MvvmLight) are either not maintained anymore, or are much bigger in scope (eg. supporting platform specific APIs too), or with an outdated codebase, or all of those things combined. Having a single entry point for the basic MVVM types in the toolkit would be a guarantee for new devs for where to go look for well tested and up to date, minimal MVVM APIs to use without 3rd party references.
  • Some of the frameworks (eg. Microsoft.Extensions.DependencyInjection) are either narrow scoped (eg. just dependency injection) or too extensive and less intuitive to use, or both. Having a minimalistic set of MVVM APIs in the toolkit would be an easy way for devs to get started, or an all-in-one solution for many devs just needing the basic types to get going. Eg. personally the classes in this PR would be all I need in my applications, and the same goes for many other devs.
  • As @hansmbakker mentioned, having the APIs being from an official package instead of having to research and add 3rd party packages is a big plus for many devs. They could just add all the Microsoft.Toolkit.* packages they need and they'd be good to go.
  • We can sync up with the devs working on the Windows Template Studio (as @sibille mentioned) so that they could integrate this new package directly into their project as well, also losing the 3rd party reference to MvvmLight in the process, which is also nice.

Regarding what @skendrot said here:

MVVM Frameworks always start out as something small. Then more and more features are added making them larger and larger. Creating a framework within the toolkit would just explode to being more and more.

This is the textbook "slippery slope argument", and I don't think we should base our decision on this. We can define the scope of the package and leave it at that, but not doing anything at all just because it could potentially grow too big in the future is not what I'd suggest. I think all the numerous advantages of this package should also be weighted in, along with the fact that as I mentioned, we could just clearly state the main points of the package and reject possible future proposal that would not go in the same direction. That wouldn't be any different from the toolkit as a whole.

All in all, the main idea here would be to have a minimalistic, easy to use, well maintained, modern, high-performance set of MVVM primitives from a 1st party package, an all-in-one toolkit for MVVM scenarios that would suit a large number of developers needing these basic building blocks.

Many of these are things that literally every developer basically needs (eg. ObservableObject, RelayCommand, etc.), and I think the package as a whole, for how it's structured right now, would be a good fit for what the toolkit represents. Of course I might be biased since I did write the thing, but I hope I raised some valid points here as well ๐Ÿ˜„

I agree with a lot of things you say. And ๐Ÿ‘ for creating this PR! There are a lot of useful classes in it and, if they are maintained in a nuget package, that allows for much easier updating than one-time code generation.

One thing I am wondering about: are there many people who do MVVM without dependency injection? If not, would there be a case for standardization towards DI? Or would too many people complain? What would that change to the ViewModelBase class?

offering static APIs so you don't necessarily need to pass instances of your container around

In what cases would it be needed to pass that container around? I thought that, since both the ViewModels and their dependencies are registered and have their dependencies injected, there is no need to pass your container around?

@hansmbakker Glad to hear that! ๐Ÿ˜„

As for that quote, it actually refers to an old version of the PR - both the Ioc and the Messenger class fully support DI now. In fact, I've also added some constructors to ViewModelBase to make it easier for people that use DI to pass those instances. At the same time, both classes also expose a static, thread-safe Default instance, so you're free to use whatever approach you prefer, either DI or static ๐Ÿ‘

As for why people would need multiple instances, an example is when running multiple app instances in the same process (eg. with multiple app windows), people would want to have an instance per window. Or, having instances to inject could be useful when writing unit tests.

As for your other question, for instance I use the static approach in some of my apps, as it simplifies the codebase when you don't need the additional flexibility of using DI.

In general, multiple people in the Discord server said they'd want to use the DI approach, so since the required code changes were minimal I added both options ๐Ÿš€

Relaying some updates here from the UWP Discord server to keep everyone up to date.

Following an extensive conversation with @Aminator and some prototyping, I went ahead and did a number of changes in #3229, specifically related to the dependency injection area:

  • Completely nuked the old Ioc class/interface.
  • The package now has a dependency on Microsoft.Extensions.DependencyInjection so we can use those types directly (eg. IServiceProvider), which make the package better for interoperability, saves us work/test/maintenance and can also act as an easy to use "entry point" for users to become familiar with the APIs from that package. @azchohfi I think you'll be happy about this since you were the one that suggested that package from the start ๐Ÿ˜Š
  • With the better integration now, the ViewModelBase class now takes the general IServiceProvider interface in its secondary constructors, so users also have the ability to do DI with their own service providers coming from somewhere else.
  • On the topic of better integration, with this change we'd now have both the toolkit and the Windows Template Studio all using the same exact DI APIs, coming from an official 1st party Microsoft package (Microsoft.Extensions.DependencyInjection).
  • The Ioc class now acts as a super easy to use service configurator for those APIs.
  • Tested this already on UWP Debug/Release as well, works just fine.

Sample usage of how to configure services with the new Ioc class:

Ioc.Default.ConfigureServices(services =>
{
    services.AddSingleton<ILogger, Logger>();
    services.AddSingleton<IPrinter, PrinterService>();
    // ...
});

The Ioc class then exposes an IServiceProvider ServiceProvider property which is automatically used by ViewModel base if no other provider is specified, which means that users can just use that code to initialize services, and then every single ViewModel in their app will automatically have the resulting IServiceProvider instance being injected. They can then just use all the extensions available from the Microsoft.Extensions.DependencyInjection package to get service instances from that provider.

This solution has the advantage of being both easy to use, highly customizable, and using the same APIs from Microsoft.Extensions.DependencyInjection while still having the same behavior from the original Ioc class from MvvmLight. Plus we don't have any code to maintain at all in that area.


Other general changes from the PR (and a few more points for @skendrot I forgot to mention the other day). Regarding having basic MVVM building blocks in the toolkit, I realized the toolkit actually already does have a few of them. In particular Singleton<T> and NotifyTaskCompletion<TResult>:

  • The Singleton<T> is scheduled for removal in 7.0 already (see #3134), but that's just for performance reasons and not lack of usage. In fact, that class was basically a minimalistic inversion of control container. With the new Mvvm package, users relying on that class will be able to easily switch to the new Ioc class without losing any functionality (and gaining the additional features).
  • The NotifyTaskCompletion<TResult> class is being replaced by the SetAndNotifyOnCompletion method from ObservableObject, which exposes the same functionalities (and more) without requiring dedicated types at all, which also makes it more efficient and makes the code easier to read. Here is how the new method can be used in a small sample:
private Task<string> myTask = Task.FromResult<string>(null);

public Task<string> MyTask
{
    get => myTask;
    private set => SetAndNotifyOnCompletion(ref myTask, () => myTask, value);
}
<TextBlock>
    <Run Text="Task status:"/>
    <Run Text="{x:Bind ViewModel.MyTask.Status, Mode=OneWay}"/>
    <LineBreak/>
    <Run Text="Result:"/>
    <Run
        xmlns:ex="using:Microsoft.Toolkit.Extensions"
        Text="{x:Bind ex:TaskExtensions.ResultOrDefault(ViewModel.MyTask), Mode=OneWay}"/>
</TextBlock>

Which results in this with a function that waits two seconds and then returns "Hello world!":

NVXa3N8Wom

Same feature as before (but in a dedicated, specialized package now), and without the additional "proxy" object, so that the property you have in your models is actually of type Task<T> ๐ŸŽ‰

Really happy to see the direction you took!

By the way, donโ€™t the view models and views need to be registered with DI, too?

(I thought that is a common practice, but I didnโ€™t pick that up from your description.)

@hansmbakker Sergio wants to explicitly not use constructor injection and instead uses the anti-pattern where you access the IServiceProvider directly in your view model. The ViewModelBase accesses a static instance of the IServiceProvider by default, that's why it doesn't need to be registered as well.

This is the opposite to what I do in my apps. I always use constructor injection in my view models and services, which is also why I don't need the Ioc or ViewModelBase classes at all. Singleton and sometimes transient view models are also part of the service provider if you use this pattern. This has the benefit that the view models and services don't know where their dependencies come from and can also be instantiated normally if needed. This is also the recommended pattern in ASP.NET.

Lastly, views are never part of the service provider because those should generally have an empty constructor to be able to instantiate them in XAML, which would interfere with constructor injection. The services a view needs should come from the associated view model instead, which is typically the DataContext property and exposed as a strongly typed ViewModel property to the view for proper support with x:Bind.

explicitly not use constructor injection and instead uses the anti-pattern where you access the IServiceProvider directly in your view model

@Aminator thank you for explaining that, I had not seen that intent / consequence!

@Sergio0694 I agree with @Aminator here, constructor injection with registered view models has the advantages that @Aminator describes, and furthermore it is a common practice (so it makes people "feel at home" quickly), why deviate from that? Is there a reason why you are doing it this way?

If this is going to be the approach that the toolkit will provide, and WTS might use this for the basic MVVM code in the future, please ensure that it promotes best practices.


views are never part of the service provider

@Aminator probably it is less common, I was not used to it too, but I saw it being done in this ReactiveUI/Uno sample (code) last week.

We had an extensive conversation about this too with @Aminator while drafting the new APIs structure, and I'm actually a bit surprised to see my approach be referred to as an "anti-pattern" now, because it really itsn't. Or at least, it might technically be if you're trying to exactly replicate the architecture of an ASP.NET app, but that's not the point of this package. This is not Microsoft.Toolkit.AspDotNet, it's .Mvvm. The MVVM pattern per se doesn't have any single "official" way of handling services, mostly because it doesn't really concern itself with how they are handled. Many people mix MVVM with other patterns like DI, but that is indeed a mix of two separate patterns. MVVM on its own is only about logically separating the view with the business logic of the application and the models, and in particular is meant to facilitate the use of features like data binding. That's really all that MVVM is in a nutshell.

That said, let me clarify a couple points on this and also answer @hansmbakker's question:

  • This has already been answered by @Aminator, views don't need to be registered. Furthermore, you really don't _have_ to register viewmodels either, regardless of what is commonly done in ASP.NET. That's not a requirement for MVVM, though sure you can do that too.
  • All that the current ViewModelBase really does is to basically replicate the functionality that the same class from MvvmLight provided. This is particularly useful for developers that go for a more "traditional" MVVM approach, without also doing DI (like me). Doing this has the advantage of making the code _much_ simpler and easier to maintain, if using this approach is possible in your specific situation. What the ViewModelBase class does in this case is simply this: "are you injecting a service provider? If so, I'll use that. If not, let me ibject the default one for you". You can see how in all cases where devs are in fact only using a single IServiceContainer (or IMessenger) instance for the whole app this incredibly simplifies things, as it lets you completely get rid of manual DI altogether. To make a practical example, here is how I usually declare a view:
<UserControl
    x:Class="MySampleApp.Views.ConsoleView"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
    <UserControl.DataContext>
        <views:ConsoleViewModel x:Name="ViewModel"/>
    </UserControl.DataContext>

    <!--Other stuff in the view here-->
</UserControl>

You can see that I'm spawning the viewmodel right from XAML - I can do this since it doesn't need any kind of DI in its constructors. This also lets me have the ViewModel field directly from there to use x:Bind, without the need of manually adding the strongly typed ViewModel property in code behind at all. In fact, with this approach I often don't need to write _any_ code behind at all. Now, that viewmodel will also automatically receive the default IMessenger and IServiceProvider instance from the ViewModelBase, which means that:

  • Code in the viewmodel can simply use the Messenger and ServiceProvider instances to send messages and resolve services, with no need to use the static instance directly. This also helps the modularization: if one day I wanted to make changes I could just inject a different instance manually without having to change the code at all.
  • The initialization that devs do at startup with Ioc.Default.ConfigureServices(...) is automatically available to all viewmodels in their app by default, without the need to write any other code.

You could say that this is not 100% "proper ASP.NET-like DI patter", and you might be correct, but it's 100% valid MVVM pattern, which is the main point of this package. Furthermore, I should add:

  • This is made possible by a single additional constructor in ViewModelBase, and if devs want to manually inject services they're free to do that too. Both approaches are entirely possible using this package and both would work fine, one is just easier to get started with.
  • As I mentioned, this is exactly the same behavior that MvvmLight was using, and that a lot of devs (especially if migrating from there) will be used to. All I did here was to also enhance that same architecture with better interoperability with the Microsoft.Extensions.DependencyInjection package, so that hopefully everyone is happy ๐Ÿ˜Š

Regarding my MVVM helpers:

  1. I would recommend taking the NotifyTask from my GitHub; its API has gotten a bit better over the years.
  2. I would not recommend using AsyncCommand. Possibly IAsyncCommand and an AsyncCommandBase, but that would be about it. There's just too much variation in what people expect for a single AsyncCommand to be a useful type. I think it makes more sense for the "async command" concept to be composed of several smaller pieces, and this is the approach I've started down in my GitHub repo, but it's still quite a way from being finished (hence the prerelease on the NuGet package), and I'm not prioritizing it at this time.

Hey @StephenCleary - thank you for chiming in!

  1. I'm not really a fan of the NotifyTask class due to the way it basically duplicates and proxies every single property, whereas we can just bind directly from XAML to all of these, without even needing the second object at all. Have you seen my code example in the second part of this message? That's the solution I came up with so far in the PR, let me know what you think!
  2. Not really sure I understand the argument against not having AsyncCommand ๐Ÿค”
    We do have the interface, it's just a matter of also providing a reference implementation ready to use - devs who are fine can just use that directly, and devs in need of more control can just reimplement their commands anyway. There's no loss of functionality either way, it's just about also having a concrete type for each interface available out of the box. Additionally, not providing a concrete type would also break the "symmetry" of the available types in the package, where right now we offer a concrete class for each interface we expose, so that devs that are ok with the base functionality don't have to reinvent the wheel on their own every time (eg. Messenger for IMessenger, ObservableObject for INotifyPropertyChanged, RelayCommand for ICommand, etc.).

I like how SetAndNotifyOnCompletion simplifies the implementation, and I love how it allows the VM property to be of type Task<T>, but I'm not wild about how it adds complexity to the end user:

  1. The indirect binding syntax is more awkward.
  2. This approach is more error-prone because the necessity of indirect binding for Result is not obvious. Your docs would all have to be very clear right from the beginning that it is not correct to bind to ViewModel.MyTask.Result and that users need to bind to ex:TaskExtensions.ResultOrDefault(ViewModel.MyTask) instead. And even then, you'll get a number of bug reports about how binding to ViewModel.MyTask.Result doesn't work.

That said, building NotifyTask<T> on SetAndNotifyOnCompletion would be straightforward, if a friendlier API was desired in the future.

Regarding AsyncCommand, it's just my general sense (from helping many, many async MVVM projects) that most people need something different, and that there is no general-purpose AsyncCommand that satisfies everyone or even a majority of people. E.g., let's say it's something like a 4-way tie between incompatible desired implementations. So whatever AsyncCommand you have will only satisfy about 25% of users, and 75% will need to implement IAsyncCommand themselves. I don't have firm numbers but I'm pretty sure there isn't an implementation that satisfies even a majority (50%) of users.

That said, your arguments for including an AsyncCommand of some kind are very valid, and I have no objection to that.

I'm really glad to hear you like how SetAndNotifyOnCompletion is structured ๐Ÿ˜Š
As for your points:

  1. It is more awkward, I agree, but thankfully that's only needed for the Result property, all the others can be bound to directly. Als I feel like as time goes by, developers should already be quite familiar with the x:Bind to function syntax at this point. And as you said, docs would also have to be very clear on that, absolutely agree.
  2. About this and the point above on the docs, I feel like this would actually be a perfect opportunity to solve multiple problems in one. Just having the Task property is better compared to having a specific type like NotifyTaskCompletion (after all, this is similar to why the #1056 proposal was rejected too, it was essentially the same just for generic T values), it's more efficient (less allocations) and makes the syntax simpler when you're not binding to Result. As for Result, I think that that's such an important topic that every developer working with async code in C# should really understand _why_ they should never use the Task<T>.Result property in their code unless they really know what they're doing, and this would be the perfect chance to make that extremely clear from the docs, so that devs would learn both how to use the new SetAndNotifyOnCompletion method properly and how to bind to Result, and possibly also learn the basics of that, so they could also avoid incurring in the same issue in other parts of their code. After all, the reason why they shouldn't bind to Result is exactly the same why they shouldn't access Result in code behind too. So I really feel like this could both simplify code, be more efficient and be a good learning opportunity for everyone, especially less experienced developers.

And of course, as you mentioned I agree it would be easy to eventually add an actual NotifyTask<T> by using that method, I just feel like for the reasons above, we might as well take this chance to avoid having to do that and improve the general API surface in that area ๐Ÿ˜„

Regarding AsyncCommand, I see what you're saying. I did try to make the basic implementations as general purpose as possible, but I definitely understand what you mean when you say there will always be devs needing to write their own commands no matter what, that's fair. I'm glad to hear you're not actually against the idea of simply providing _some_ basic implementation too though, that's great ๐Ÿ˜Š

Alright, so after another conversation with @Aminator I proceeded to make another small refactoring (https://github.com/windows-toolkit/WindowsCommunityToolkit/pull/3229/commits/86d3b49f5b3175f1c27d64efc8036abe434aea2f). The Ioc.ServiceProvider property is now faster as it doesn't lock anymore, and I've removed the IServiceProvider property from ViewModelBase, so that the resulting implementation should now better follow the suggested DI pattern from ASP.NET, and also be closer to the implementation from MvvmLight. For devs relying on the service locator pattern instead of DI (including me), the same exact functionality is still available, just through the default service provider instead of a local property:

Ioc.Default.GetInstance<IFoo>().DoSomethingCool();

This works because Ioc now directly implements IServiceProvider, and relays calls from the extensions from Microsoft.Extension.DependencyInjection to the internal ServiceProvider instance. This means that users can directly use ASP.NET extension methods on the Ioc instance, without even having to go through the ServiceProvider property at all anymore.

Hopefully this makes everyone happy, as the new solution is more adherent to the guidelines for DI, while still being similar to MvvmLight and offering the flexibility of using both approaches (service locator or proper DI) to developers either migrating from MvvmLight or building a new app ๐Ÿ˜Š

Really appreciate everyone's thoughts and support on this topic! ๐Ÿฆ™โค ๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰

I've been heads down with our upcoming 6.1 release, but I'll plan to circle-back on this in June. The biggest open concerns are how this plays in the ecosystem at large and to make sure we can maintain the code and the ideology of leanness here and not just have this continually grow in scope.

I've been thinking it may make the most sense to at least have this as part of a 7.0 preview package to broaden the scope of folks being able to try this out on their own and gather more feedback.

This could also help with conversations with other groups about how this came to be and why having a 'reference' implementation in the toolkit would be important, lower the barrier to entry, but also give them freedom to continue the advanced topics they handle best.

Is there an official statement about an end of support for MVVMLight anywhere? I've not seen anything. Seeing something from Laurent would really help understand the justification/motivation here.
I was of the understanding that MVVMLight was "Done, not dead." It was meant to be a minimal set of building blocks to follow the MVVM pattern. It was never meant to be a fully-fledged, all-singing-all-dancing framework. It's had this for years and so doesn't need active development.
The impression I get from this issue is that due to wanting more than what it offers and because it's not being actively developed a replacement should be created.

If this is intended as a replacement for MVVMLight how/should the lightweight principle of the original be preserved?
Will anyone wishing to switch from MVVMLight be guaranteed that they can start with just a namespace change?--Requiring more than this might be hard to justify.

Hey @mrlacey - thank you for chiming in!
I'll try to go through the various points you made as best I can.

"Is there an official statement about an end of support for MVVMLight anywhere?"

I haven't seen an official statement per se, but you can see that the latest commit is from late 2018, and Laurent himelf has commented on this MVVM package on Twitter (here), saying he liked the idea and also mentioning how he doesn't have time to work on MvvmLight anymore.

" It was meant to be a minimal set of building blocks to follow the MVVM pattern. It was never meant to be a fully-fledged, all-singing-all-dancing framework."

The same goes for this MVVM package. You can see that it isn't an all around, complex framework like eg. Prism, it's basically a "reference" implementation of the basic MVVM primitives, in a package that's up to date, modern, lightweight, minimal and efficient. You can see the Key Tenants that @michael-hawker added to the first post in this issue for the main points about this.

"The impression I get from this issue is that due to wanting more than what it offers and because it's not being actively developed a replacement should be created."

It's not really about "wanting to do more" - there are some key differences between MvvmLight and this package. I'll try to briefly go through them to hopefully help clear things up on this point:

  • This package only targets .NET Standard 2.0, with no platform/runtime-specific code. This makes it extremely modular and reusable, with the codebase being extremely easy to maintain as well, as there's no precompiler directives and special includes to manage. This also makes it much easier to leverage more modern APIs when needed (eg. Span<T>).
  • For the reason above, we can just target .NET Standard 2.0 as a baseline and avoid having to compromise in performance due to missing APIs on older framework. Also, since this is a new package, we can make "breaking changes" compared to MvvmLight, like changing the API surface in the Messenger class. And you can see the benefits of this - in the benchmarks I posted in the first post, the new Messenger is about 8x faster and up to 2.000.000x (!) more memory efficient.
  • Similarly to the above, we can also use this occasion to improve the interoperability of this package with existing, 1st party libraries. In particular, compared to MvvmLight which includes its own implementation of a DI container, this library relies on the Microsoft.Extensions.DependencyInjection package, providing an easy to use entry point for new developers. This both saves the maintenance time for this package, and lets users use the full power of that DI container, which is _extremely_ feature rich, well tested and well known. This also means that the DI provider offered by this package will use common BCL interfaces (IServiceProvider), improving the synergy with the rest of the entire ecosystem as well.

"If this is intended as a replacement for MVVMLight how/should the lightweight principle of the original be preserved?"

The lightweight principle is still present. Actually, I'd argue this is even more lightweight in principle, considering the pretty big performance gains and memory efficiency improvements compared to MvvmLight. As far as being simple, as mentioned above this package isn't supposed to be a full "middleware" like Prism or similar libraries, but rather a set of basic APIs to get started with MVVM and related patterns. You can see the full list of available APIs in the first post, which is not at all different than the ones provided in MvvmLight. Actually, here there are slightly less APIs, as we removed some platform-specific ones (eg. a navigation service), to make the package truly platform agnostic.

"Will anyone wishing to switch from MVVMLight be guaranteed that they can start with just a namespace change?--Requiring more than this might be hard to justify."

Switching from MvvmLight will not be just a namespace change, due to some API differences, but I've already done this in one of my apps as a test bench (and @Aminator did the same in his DX12GameEngine project as well), and moving to this package was extremely straightforwards in general. In many cases it really was just a matter of changing namespaces and possibly tweaking a thing or two. Of course, I'll provide a detailed get started guide with an introduction to all the available APIs, as well as code samples that will show how to use every bit of this new package ๐Ÿš€

Additionally, I don't agree that the hypothetical constriction of offering the same exact API surface just to make the transition slightly easier would be worth it. As I said, this would be the perfect time to make some breaking changes that will be hugely beneficial in the long run, such as switching from yet another custom DI container, to something like Microsoft.Extensions.DependencyInjection, which also comes with advanced features that many developers might want to use, as well as improving the interoperability in general with other libraries, as it uses common BCL interfaces. Or, even just for a performance perspective, I think it's worth it to have some slight changes if you get a major performance/memory efficient boost in return.

Hope this helps! ๐Ÿ˜Š

I've uploaded a preview package that can be used locally to try out the new APIs for anyone interested, as suggested by @michael-hawker. Updated the first post, reposting here to notify subscribers:

Preview package (built out of https://github.com/windows-toolkit/WindowsCommunityToolkit/pull/3229/commits/c6b4cb652de8a563c0a727d43ff11f344655624e): download here.

@Sergio0694 - thanks for this, simple cases on messaging are working really well. One question, does the messenger allow calling async methods on the receiver?

For example, the code below gives the following error on line string response = Messenger.Default.Send<MsMVVMRequestMessageEvent>();

System.InvalidOperationException
  HResult=0x80131509
  Message=No response was received for the given request message
  Source=Microsoft.Toolkit.Mvvm

Event:

public class MsMVVMRequestMessageEvent : RequestMessage<string> { }

ViewModel1:

Messenger.Default.Register<MsMVVMRequestMessageEvent>(this, async m =>
{
    string s = await TestMsMVVMMessaging();
    m.ReportResult(s); // Assume "CurrentUser" is some local var/field/property
});

public async Task<string> TestMsMVVMMessaging()
{
    await Task.Delay(5000);
    return DateTime.UtcNow.ToString("yyyy MMM dd HH:mm:ss.fff");
}

ViewModel2:

private void GetData()
{
    string response = Messenger.Default.Send<MsMVVMRequestMessageEvent>();
    Data = response;
}

Is there a way to make this work currently?

It would also be really nice to see some documentation and examples in the future ๐Ÿ‘

Hi @cirrusone - glad to hear you're already trying this out! ๐Ÿ˜„

The reason why you're seeing that error is that as the callback is asynchronous, the sender continues before that async method completes and thinks no response has been returned (which is true, no response has been returned _yet_ at that point).
The whole messenger class is synchronous, but you should be able to work around this by simply making the actual return type of your request message a Task<T>. That is, consider the following example:

// "Async" request message, returns a Task<T>
public sealed class MyAsyncRequestMessage : RequestMessage<Task<string>> { }

// Subscribe to the request
Messenger.Default.Register<MyAsyncRequestMessage>(this, m =>
{
    m.ReportResult(TestMsMVVMMessaging());
});

// Request and wait for the response
string response = await Messenger.Default.Send<MyAsyncRequestMessage>().Result;

The key here is that since the Messenger APIs are all synchronous, you need to _synchronously_ set the resulting Task<T> in the request message (even if that hasn't completed yet, that's totally fine). This way the messenger will see that a response has been returned, and will just relay the Task<T> to await to the original sender, like in the example above.

Also note that in this case you do need to explicitly use .Result on the returned message - the C# compiler isn't able to use the implicit converter automatically when you're using await in front of that, so you need to give it a little hint there ๐Ÿ˜Š

As for the docs, they're currently work in progress as the package hasn't been officially approved yet, but we'll probably start looking into this a bit more starting from next week. I'll publish them as soon as I have a draft I can share, absolutely!

Thanks again for your interest! ๐Ÿš€

Thanks for the example it's working.

I thought awaiting on .Result was blocking?

Perhaps it's related but using the async example with the receiver in the MainWindow.xaml / MainWindowViewModel.cs, if I open a child window whilst awaiting the result using something like _dialogService.Show(nameof(UserControlMVVMChildWindow), new DialogParameters(), null); the next time I call string response = await Messenger.Default.Send<MsMVVMRequestMessageEvent>().Result; results in the following error

System.InvalidOperationException: 'A result has already been reported for the current message'

System.InvalidOperationException
  HResult=0x80131509
  Message=A result has already been reported for the current message
  Source=Microsoft.Toolkit.Mvvm

Sample project

Steps to recreate:
1) Click Launch
2) Click Launch Child from Main VM
3) Click Get Data From MainViewModel Best way using new MS MVVM library
4) Before Result from 4) is returned in 5s, click Launch Window from MainViewModel injecting variables
5) Close new window
6) Repeat 3) gives error

"I thought awaiting on .Result was blocking?"

It is, but that .Result there is _not_ the Task<T>.Result property, it's the RequestMessage<T>.Result property! ๐Ÿ˜„
As in, you're getting the resulting response value, which is a Task<T>, and then awaiting that one.

As for the error, as I said that happens if the request message doesn't synchronously receive a response. Be careful not to confuse the usage of sync/async code here: the fact that you're invoking an async method is completely irrelevant here, all that matters for the messenger is that the Task<T> result is set _synchronously_ within the message handler. Also, make sure to register handlers _before_ sending request messages, otherwise they just won't be able to send a reply at all.

I'll see if I can take a look at that repro in a bit ๐Ÿ‘

"I thought awaiting on .Result was blocking?"

It is, but that .Result there is _not_ the Task<T>.Result property, it's the RequestMessage<T>.Result property! ๐Ÿ˜„
As in, you're getting the resulting response value, which is a Task<T>, and then awaiting that one.

Should we think about a different property name or something?

@Sergio0694 let's start working on docs and samples, anyone who uses tech other than UWP that wants to help us is more than welcome to contribute as well! I'm thinking we can create a separate repo in the org here specific to MVVM samples and where we can host some of the initial documentation before release, thoughts?

"I thought awaiting on .Result was blocking?"

It is, but that .Result there is _not_ the Task<T>.Result property, it's the RequestMessage<T>.Result property! ๐Ÿ˜„
As in, you're getting the resulting response value, which is a Task<T>, and then awaiting that one.

Should we think about a different property name or something?

Whoops, forgot to update my reply here! I've refactored those APIs quite a bit in a previous PR, they're way less verbose and more user friendly now when specifically dealing with async request messages, in fact there's a dedicated message type now.

Here's how it works:

// "Async" request message, returns a Task<T>
public sealed class MyAsyncRequestMessage : AsyncRequestMessage<string> { }

// Subscribe to the request
Messenger.Default.Register<MyAsyncRequestMessage>(this, m =>
{
    m.Reply(GetSomeStringAsync());
});

// Request and wait for the response
string response = await Messenger.Default.Send<MyAsyncRequestMessage>();

The trick here is that AsyncRequestMessage<T> exposes a GetAwaiter method, so you can now just use await on it directly, and it'll just relay that to the actual Task<T> response contained within the message for you ๐Ÿ˜Š

This will all be properly documented in the docs of course!

Hey everyone, as @michael-hawker suggested I've linked a (very) draft preview of the docs in the first post ๐Ÿ˜Š

Also including the link here for extra visibility: https://gist.github.com/Sergio0694/987209d0855837ee6835f6e758f5cf40.

Hey everyone, as @michael-hawker suggested I've linked a (very) draft preview of the docs in the first post ๐Ÿ˜Š

Also including the link here for extra visibility: https://gist.github.com/Sergio0694/987209d0855837ee6835f6e758f5cf40.

Is there a version that makes it easy to give inline feedback on? (say as part of a PR?)

@mrlacey I'm working on a draft in a branch on the sample repo, I'll open a PR there as soon as it's finished so you'll be able to just add review comments directly on each line or section you think needs some work or changes ๐Ÿ‘

I've been looking at how the Calcium framework includes a built-in exception handler for exceptions thrown within messages. Has anything similar been considered/discussed for inclusion here too? I couldn't find anything but this strikes me as a possible nice addition.

N.B. I haven't considered how this might work but just thinking about possibilities.

is this feature ready to use? I tried install Microsoft.Toolkit.mvvm to my .net framework 4.7.2 application but nuget cannot search such package.
Which package in Microsoft.Toolkit is equivalent of mvvmlight?

is this feature ready to use? I tried install Microsoft.Toolkit.mvvm to my .net framework 4.7.2 application but nuget cannot search such package.
Which package in Microsoft.Toolkit is equivalent of mvvmlight?

This is currently only available in preview (from the MyGet source)
It will be released as part of version 7.0

Documentation on migrating from MvvmLight is currently in progress.

Hey @LeiYangGH - the feature is ready but it's still in early preview for now, so the package is only available on MyGet. If you'de like to try it out, you need to create a nuget.config file next to your .sln project, with this content:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <add key="ToolkitMyGet" value="https://dotnet.myget.org/F/uwpcommunitytoolkit/api/v3/index.json"/>
  </packageSources>
</configuration>

Then you need to go to the NuGet package manager page for your project and ensure that the ToolkitMyGet feed is selected. From there, you should be able to find the Microsoft.Toolkit.Mvvm package and install it ๐Ÿ‘

You can refer to the preview docs that are in the first post for this issue.
We'll also be providing more detailed docs and samples shortly ๐Ÿ˜„

EDIT: ah, @mrlacey beat me to it by 7 seconds ๐Ÿคฃ

thanks a lot! your response is the fastest i've ever seen in github. i'll try myget.

Hello,
have you plan to add support of cancellation to async command (New class CancelAsyncCommand or add to existing implementation) ?
Adding property like IsBusy/progress or whatever would be great too.

We've considered it, but it seemed like too much functionality for a base implementation to add to the library at least for now. It should be relatively easy to write a custom command extending the async command interface, to also add cancellation support. Of course, it's certainly doable to also just add support for this to the IAsyncRelayCommand interface, though we'd need more feedbacks on this to decide to go forward with this change.

As a reference, with this change the interface would be something like this:

namespace Microsoft.Toolkit.Mvvm.Input
{
    public interface IAsyncRelayCommand : IRelayCommand, INotifyPropertyChanged
    {
        Task? ExecutionTask { get; }
        bool IsRunning { get; }
        bool IsCanceled { get; }

        void Cancel();
        Task ExecuteAsync(object? parameter);
    }
}

And I'd probably make the constructor take a Func<CancellationToken, Task> instead of just Func<Task>, so you'd be able to either just do _ => { ... } if you don't care about cancellation, or just use it within your code as needed. Might be a bit less handy to use for folks using the C# method group syntax though if they were closing over parameterless async methods. But then again this in turn would enable support for doing the same with ones accepting a token. I guess we can have an entire conversation on this suggestion ๐Ÿ˜„

Pinging @XamlBrewer @jamesmcroft @mrlacey


>>> Further discussions in https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3428 <<<

This is the issue tracking all new changes to the MVVM Toolkit for the next preview of the library.
This issue only referred to the initial preview version and is not tracking the current state of the library.

Was this page helpful?
0 / 5 - 0 ratings