PrismLibrary / Prism

Prism is a framework for building loosely coupled, maintainable, and testable XAML applications in WPF, Xamarin Forms, and Uno / Win UI Applications..
Other
6.27k stars 1.64k forks source link

[Win10 UWP] Changing patterns with {x:Bind} #58

Closed apollyonbob closed 9 years ago

apollyonbob commented 9 years ago

So {x:Bind} is changing how binding works. {Binding} connects to a context, which at a top most level is a DataContext. {x:Bind} doesn't connect to a context, it connects directly to properties on the host control, and is strongly typed. This means that {x:Bind} can't use DataContext, because DataContext is type object. Also, as {x:Bind} has much better performance than {Binding} it should be the new default going forward.

With that context, I was just wondering how this should be handled. I mean I know in the short term one could just make a property that returns (IViewModelType)DataContext, and hope it all kind of works out haha. But in the long term, is there a plan for what the IView should look like to accommodate this?

Some thoughts - you could specify the ViewModel Type you're looking for in the IView itself - IView I suppose. And IView could also implement IView, so you can still do the IView checking for AutoWire?

brianlagunas commented 9 years ago

Actually, in Prism 6 IView is gone. There will be no need for it. As far as using x:Bind, you simply need to add a strongly typed property to your View code behind and use x:Bind against that.

public MyVieModel ViewModel { get { return (MyViewModel)DataContext;} }

bartlannoeye commented 9 years ago

While x:Bind brings a nice performance boost, it sadly breaks the separation between view and view model that MVVM brings. Not sure if we should add code to enable this scenario.

The strongly typed property is indeed the easiest way to make it work. We could add a code snippet to the prism releases to help users create the necessary property.

Mike-E-angelo commented 9 years ago

public MyVieModel ViewModel { get { return (MyViewModel)DataContext;} }

Yuck. :stuck_out_tongue_closed_eyes: (Not you, Brian, just the nature of the matter.)

While x:Bind brings a nice performance boost, it sadly breaks the separation between view and view model that MVVM brings. Not sure if we should add code to enable this scenario.

I agree. What is going on over there in the UWP group? I am not sure if you saw this, but they actually admitted during this Channel9 session that they were considering removing Bindings from Xaml altogether as an actual legitimate solution before landing on their "new and improved" x:Bind. Even after all this time, they still don't have markup extensions (or about 80% of what makes Xaml so awesome). As you (sort of :smile:) allude to, Bart... does anyone in that group know (or care) what MVVM is or are they just making it up as they go along? The whole WinRT/Windows Store platform feels hijacked compared to WPF/Silverlight. That's exactly the word it feels... hijacked, and by all the wrong engineers. Right down to that blasted Curtain of COM that hides all the particulars so you can see what is really going on in the application.

(Sorry. Just had to rant a bit. It's been a while! Haha.)

Terrible. Just terrible. And frustrating. Is it too late to start (another) Silverlight6 petition? Doh.

apollyonbob commented 9 years ago

Duck typing (i.e. current Bindings) isn't separation though - it's just obfuscation. If you change the underlying type, it still breaks if it doesn't match the implicit interface. If you bind x:Bind to an interface instead of a concrete type, you've maintained the dependency inversion principle, and removed the obfuscation. It doesn't really affect MVVM because MVVM just describes the relationship between the objects, not how they're connected. And it adheres to SOLID principles better.

But I definitely agree that wrapping the DataContext in the property seems like a hack because you are losing and then trying to regain type information. I mean it's true that most of the time I'm sure it's probably fine - as long as you set that DataContext prior to the Bindings being loaded. (Which, presumably, Prism will do.) But losing that type info by passing through DataContext does feel wrong.

But I mean, it's a wrongness I can accept for now at least haha.

brianlagunas commented 9 years ago

Does it work property with interfaces? Off the tope of my head, we may be able to look into an IView[T] with a definition like this:

public T ViewModel {...}

This would be a UWP thing only and not included in the other platforms for obvious reasons. Of course this would be optional, but I am not really sure exactly what this would be buying you since you are basically still having to create a property in the code-behind. So I'm not really seeing the benefit off the top of my head.

apollyonbob commented 9 years ago

Yeah x:Bind doesn't care what the target is, it's just strongly typed.

Your right in that we'd have to make a property no matter what, my only concern was that losing typing through DataContext feels bad.

Like, for proper injection, you need some place to put the properly typed object. Usually this is done through the constructor, but we can't cuz of the framework. The backup injection site is usually some kind of property. But if you're not planning on changing the ViewModel injection, I mean, I guess it's probably not worth it.

But yeah, IView with a IMyViewModel ViewModel {get; set;} feels a lot better than (IMyViewModel)DataContext, to me. But yeah that could be more trouble than it's worth.

brianlagunas commented 9 years ago

Well, I don't see an issue adding an IView for x:Bind support. The VML can look for that interface, and if it finds it then use it, if not, then just set the data context. Though, maybe we should think of a better name for the interface to make it more explicit on what the interface is used for. IBindable?

bartlannoeye commented 9 years ago

x:Bind works perfectly with interfaces. But then we're talking off a IMyViewModel, as the properties you're binding on have to be present on the interface since this checking is done at compile time.

If you're using IView to get the type of the viewmodel into the backing property, there's no real need to use an interface for the viewmodel.

If we plan to somehow support it (and not leave the developer to think it out), then it comes down to how will we support it: provide a code snippet, provide a project item template for the view with the field, ... (alternatives?)

On a sidenote: @Michael-DST it's been long known that the WinRT/UWP team isn't providing us with a go-to example for decent MVVM. Just check how they implemented a 'viewmodel' in the Win8 templates :smile: .

apollyonbob commented 9 years ago

@brianlagunas IBindable is usually used for binding targets, like ViewModels and Models, isn't it? IBindableView? IContextualView? IInjectableView? Ooh I kind of like that one.

@bartlannoeye Well, when it comes to "do you want the error at runtime" vs "do you want the error at compile time" I vote for compile time every day of the week, and twice on Sunday. I mean, yes, you can't just assign any old object and hope it all works out, but I consider this a feature, myself haha. But I tend to only connect things via interfaces to keep dependency inversion, and to allow for better testing. Usually. With Views, yeah I getcha, there's not a lot of reason to keep it separate from the view model haha.

But if you just provide the interface, I mean that pretty much does the rest. If you have a IInjectableView, then when they take the interface, and implement it with the default, it should produce TViewModel ViewModel {get; set;} right?

bartlannoeye commented 9 years ago

Exactly, just wanted to make sure we're all on the same page here, no matter how we try to solve it, the type in the property has to be the actual view model or it's exact interface (and not a generic base type/interface).

brianlagunas commented 9 years ago

Well this is good timing for this discussion since I am just now starting to look at UWP support, and ViewModelLocator will be the first thing that will be brought over. A better interface name will definitely need to be thought of. ISupportXBind?

briannoyes commented 9 years ago

I don't see what us adding an interface is going to buy. If you want to use x:Bind, you have to give up some decoupling and expose a property that is your ViewModel type from your code behind. Done in one line of code. What does adding an interface help with other than adding unnecessary complexity?

apollyonbob commented 9 years ago

The interface is about eliminating the loss of type information that happens when you inject a ViewModel into the DataContext, especially given x:Bind no longer tolerates this loss.

As I said earlier, it's true that a large majority of the time, this won't have adverse consequences. But for some devs like @Michael-DST above and myself, a hard cast of (ViewModel)DataContext is seen as - well distasteful is perhaps the best word - because it makes run-time assumptions.

apollyonbob commented 9 years ago

@brianlagunas Just a personal preference, but I like names that describe what the thing is meant to be, rather than why it's meant to be ... hrm ... the interface describes a View that can have a ViewModel injected in it directly. IViewModelDependent? I like how this one describes the Type you're probably going to be giving to it, i.e. IViewModelDependent tells you that T is a view model. IDependentView? Names are hard haha.

brianlagunas commented 9 years ago

I am thinking something more along the lines of IXBindViewModel. I like things that are self-describing in what they are used for. I'm still not 100% sold on this, but this is something I will play around with as it is not difficult to implement.

Mike-E-angelo commented 9 years ago

Names are hard haha.

Yes they are. Just ask PrismEvent, erm PubSubEvent. :wink:

I don't have an easy answer here either, but I do like the paths of @apollyonbob's suggestions. I would like to point out that at the end of the day, "x" is merely a prefix for a namespace. It is entirely possible that some developers might choose to use another prefix altogether for that namespace, assuming that the fine folks in the UWP group allow their "Xaml" parser to account for such creative freedom (sigh).

How about IHaveAViewModel? I kid I kid...

brianlagunas commented 9 years ago

Actually after thinking about it, I'm not sure this would work anyways. I couldn't use a generic interface, because I would need to be able to cast the View as a generic interface, but I couldn't because the ViewModelLocator doesn't have access to those types.

I would nee to be able to do something like:

var bindable = view as IXBindViewModel; ....

Of course reflection could be used, but then we are adding a performance hit for the sake of a single property.

apollyonbob commented 9 years ago

Oh yeah, that was why I was thinking it might be not worth the effort. I'm using an IOC container as a ServiceLocator to get the ViewModel from just the Type, so I only need to get the TypeInfo, and it only happens 6 times in the lifetime of the app so I think the perf hit isn't too bad. But I don't know what you're doing in your framework obv, haha, or if you can even do that.

brianlagunas commented 9 years ago

Actually, it doesn't have to do with using a container, it has to do with trying to cast a DependencyObject (the view) as a generic interface in order to set the generic ViewModel property.

TioLuiso commented 9 years ago

I tried to use a Generic Base View so that I can pass the type (or better, interface) of the VM it has to work with, so that it can expose a typed property to be used with x:Bind.

However, when trying to create an instance of the View, specifying x:TypeArguments as the VM interface, it doesn't build, and it complains that I must pass a generic parameter. And in the generated class created from the XAML, it really doesn't use the generic parameter I passed.

Has anyone tried to create a generic view in UWP? Am I missing something?

Awesome work, by the way. Just love Prism.

brianlagunas commented 9 years ago

I do not believe this is possible in UWP.

brianlagunas commented 9 years ago

Have you tried having a non-generic base class that looks like this:

public class ViewBase : Page { public IViewModel ViewModel { get { return (IViewModel)DataContext; } } }

apollyonbob commented 9 years ago

@TioLuiso Another alternative - the solution that I ultimately went with - was to have my views implement a generic interface, IViewModelDependent<T>. (Having a single property, T ViewModel {get;set;})Then when my navigation service is first created, it gets the generic property from that interface from the View. Afterward, when someone navigates to that View, it can set the ViewModel to that property. So per page, I've got 1 GetTypeInfo, 1 GetRunTimeProperty and no magic strings. And when they actually navigate I'm just calling Property.SetValue.

But my navigation service is a set of commands, so all the View Type information is known ahead of time. Despite the little bit of reflection, I'm still not certain I'd want to do any of it during a Navigation call. Because of the way I've set it up, I can do it all during app load.

brianlagunas commented 9 years ago

The best extension point to hook into with Prism would be the ViewModelLocator. Create your own ViewModelLocator attached property based off of Prism's implementation:

https://github.com/PrismLibrary/Prism/blob/master/Source/Windows10/Prism.Windows/Mvvm/ViewModelLocator.cs

You can see how the DataContext is being set in the Bind method. Just modify the Bind method to check for your custom interface and set it accordingly.

TioLuiso commented 9 years ago

Wow. Really impressive. 5 answers in 2 hours!!!

Ok @brianlagunas (Regarding the generic view) Yup. Have walked that same road. And We both found out that it didn't work. Regarding the non-generic base page: yup. It would work. However, I was trying to find some more general solution to the problem, instead of having to update just every view code behind.

@apollyonbob yup, I guess it would work, I'll take a look. Thanks a huge lot

And after all, if it doesn't prove to be a benefit big enough, I always have dynamic bindings.

TioLuiso commented 9 years ago

@apollyonbob yup. It Works. Now I'll take measurements of performance, and can take a decisión. Thanks again!!!

lock[bot] commented 4 years ago

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