Skip to content
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

[Bug]: Inconsistent BindCommand behaviour when reusing a View with a new ViewModel #3970

Open
xackus opened this issue Feb 18, 2025 · 3 comments
Labels

Comments

@xackus
Copy link

xackus commented Feb 18, 2025

Describe the bug 🐞

When using BindCommand, the ViewModel passed to the withParameter expression can be different from the ViewModel the command is invoked on.

Step to reproduce

  1. Use a ReactiveUI.Winforms.ViewModelControlHost with CacheViews = true.
  2. Set a ViewModel.
  3. Set a new ViewModel of the same type.
  4. The command parameter is computed from the old ViewModel.

Reproduction repository

https://github.com/xackus/ReactiveUiBindCommandRepro

Expected behavior

The ViewModel passed to the withParameter expression is the same as the ViewModel the command is invoked on.

Screenshots 🖼️

No response

IDE

Visual Studio 2022

Operating system

Windows

Version

11

Device

PC

ReactiveUI Version

20.1.63

Additional information ℹ️

BindCommand internally uses Reflection.ViewModelWhenAnyValue for the subscription, which despite taking a ViewModel parameter, does not use it.
The only place where the ViewModel passed to BindCommand is actually used is when invoking the withParameter expression.

I find it very confusing that the Bind*(View, ViewModel, ...) family of functions seems to sometimes use the ViewModel that was passed in, and sometimes instead extracts the ViewModel from the View.
In my case I need the second behavior, because I reuse the View when possible. But then why pass in the ViewModel at all?
I haven't been using ReactiveUI for very long. Was the approach changed here some time ago?

@xackus xackus added the bug label Feb 18, 2025
@xackus
Copy link
Author

xackus commented Feb 18, 2025

I think I found another bug while researching this issue:

public static IReactiveBinding<TView, TProp> BindCommand<TView, TViewModel, TProp, TControl>(
this TView view,
TViewModel? viewModel,
Expression<Func<TViewModel, TProp?>> propertyName,
Expression<Func<TView, TControl>> controlName,
string? toEvent = null)
where TView : class, IViewFor
where TViewModel : class
where TProp : ICommand =>
_binderImplementation.BindCommand(viewModel, view, propertyName, controlName, toEvent);

Idk how this passes the type checker, but toEvent seems to be forwarded as the withParameter parameter.

@anaisbetts
Copy link
Member

anaisbetts commented Feb 18, 2025

which despite taking a ViewModel parameter, does not use it.

The reason it does this is because it lets the C# compiler infer the TViewModel type for Expression<Func<TViewModel, TProp?>> propertyName, otherwise you would have to explicitly specify it and that'd be Annoying. The reason we specify a Func to get the ViewModel usually, is so that in the View, you can Replace the ViewModel and have everything still work

@xackus
Copy link
Author

xackus commented Feb 18, 2025

Makes sense! Thanks for the explanation.
I don't think this is mentioned anywhere in the docs. It definitely confused me. Maybe I'll make a PR later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants