search

StephenCleary / Mvvm

MVVM helpers, including calculated properties and asynchronous notification tasks.
MIT License
134 stars 38 forks source link

CanExecute not firing on OnCanExecuteChanged() #7

Closed ksachdeva closed 7 years ago

ksachdeva commented 8 years ago

Hi Stephen,

I tried the AsyncCommand in a Xamarin Forms project and saw strange behavior.

Problem : I expected my canExecute delegate to be invoked but it does not.

I read the source code of AsyncCommand and then traversed up the chain in Mvvm.Core to see the implementation of WeakCanExecuteChanged. Everything looks fine.

I then copied your AsyncCommand implementation locally, renamed it but still kept the same implementation i.e. it relies on WeakCanExecuteChanged from Mvvm.Core. The behavior was still the same.

I then copied your WeakCanExecuteChanged implementation locally, renamed it and used it with AsyncCommand. Still the same behavior. Since now I could debug I can see that OnCanExecuteChanged in WeakCanExecuteChanged is never being invoked.

I then replaced the WeakCollection with a List (essentially not using WeakReferences at all) and then it seemed to work.

I know that the RelayCommand implementation of MvvmLight uses the similar technique of using WeakReference and it does work (it is what I am trying to replace in my relatively larger project with AsyncCommand after reading your excellent articles).

Any suggestions.

Regards & thanks Kapil

StephenCleary commented 7 years ago

It looks like the same problem as this bug.

StephenCleary commented 7 years ago

And this one.

It appears as though different platforms (specifically Xamarin) have ignored the special behavior of CanExecuteChanged, instead deciding to treat it like any other event. I'm undecided as to whether this is "right" or "wrong" for Xamarin to do (arguably, the special behavior is only due to a bug in WPF kept for backcompat reasons).

Regardless, it looks like the only proper fix is to have an implementation toggle at the application level. Since my library doesn't (and won't) use platform enlightenments, it'll have to be done in code. I will, however, default to the implementation that works for Xamarin and breaks WPF.

StephenCleary commented 7 years ago

Fixed in 1.0.0-eta-05