Skip to content

Commit b9b80bb

Browse files
xackusChrisPulman
andauthored
Fix BindCommand passing incorrect parameter after assigning new ViewModel to View (#4324)
**What kind of change does this PR introduce?** Fixes #3970. **What is the current behavior?** The command parameter is derived from the original ViewModel passed into `BindCommand`. **What is the new behavior?** The command parameter is derived from the current ViewModel (`IViewFor.ViewModel`). **What might this PR break?** Theoretically this could break code that relies on the stale command parameter value. **Please check if the PR fulfills these requirements** - [X] Tests for the changes have been added (for bug fixes / features) - [X] Docs have been added / updated (for bug fixes / features) **Other information**: --------- Co-authored-by: Chris Pulman <chris.pulman@yahoo.com>
1 parent d062b37 commit b9b80bb

6 files changed

Lines changed: 147 additions & 184 deletions

File tree

src/ReactiveUI/Bindings/Command/CommandBinder.cs

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -32,26 +32,30 @@ static CommandBinder() => _binderImplementation = AppLocator.Current.GetService<
3232

3333
/// <summary>
3434
/// Binds a command from the view model to a control on the view, enabling the control to execute the command with a
35-
/// parameter when triggered.
35+
/// specified parameter when an event is raised.
3636
/// </summary>
37-
/// <remarks>This method uses reflection to dynamically observe events and properties on the control,
38-
/// which may be affected by trimming in some deployment scenarios. The binding remains active until the returned
39-
/// IReactiveBinding is disposed.</remarks>
40-
/// <typeparam name="TView">The type of the view implementing the IViewFor interface.</typeparam>
41-
/// <typeparam name="TViewModel">The type of the view model containing the command property.</typeparam>
42-
/// <typeparam name="TProp">The type of the command property, which must implement ICommand.</typeparam>
37+
/// <remarks>
38+
/// <para>The binding enables the control to execute the command when the specified event is raised,
39+
/// and automatically manages the enabled state of the control based on the command's CanExecute state.</para>
40+
/// <para>This method uses reflection to observe events and properties on the control, which may be
41+
/// affected by trimming in some deployment scenarios.</para>
42+
/// </remarks>
43+
/// <typeparam name="TView">The type of the view that implements the IViewFor interface.</typeparam>
44+
/// <typeparam name="TViewModel">The type of the view model containing the command to bind.</typeparam>
45+
/// <typeparam name="TProp">The type of the command property on the view model. Must implement ICommand.</typeparam>
4346
/// <typeparam name="TControl">The type of the control on the view to which the command will be bound.</typeparam>
44-
/// <typeparam name="TParam">The type of the parameter passed to the command when it is executed.</typeparam>
45-
/// <param name="view">The view instance to which the command will be bound. Cannot be null.</param>
46-
/// <param name="viewModel">The view model instance containing the command property. May be null if the view is not currently bound to a
47-
/// view model.</param>
47+
/// <typeparam name="TParam">The type of the parameter passed to the command when the event is raised.</typeparam>
48+
/// <param name="view">The view instance containing the control to which the command will be bound. Cannot be null.</param>
49+
/// <param name="viewModel">The view model instance containing the command to bind. Used for type inference.
50+
/// Can be null if the binding should be established without an initial view model.</param>
4851
/// <param name="propertyName">An expression identifying the command property on the view model to bind. Cannot be null.</param>
4952
/// <param name="controlName">An expression identifying the control on the view to which the command will be bound. Cannot be null.</param>
5053
/// <param name="withParameter">An observable that provides the parameter to pass to the command when it is executed. Cannot be null.</param>
51-
/// <param name="toEvent">The name of the event on the control that triggers the command. If null, a default event is used based on the
52-
/// control type.
54+
/// <param name="toEvent">The name of the event on the control that triggers the command execution. If null, a default event is used based
55+
/// on the control type. If the specified event does not exist on the control, an exception may be thrown at runtime.
5356
/// NOTE: If this parameter is used inside WhenActivated, it's important to dispose the binding when the view is deactivated.</param>
54-
/// <returns>An IReactiveBinding instance representing the active binding between the command and the control.</returns>
57+
/// <returns>An IReactiveBinding{TView, TProp} representing the established binding between the command and the control.
58+
/// It will remain active until disposed.</returns>
5559
[RequiresUnreferencedCode("Dynamic observation uses reflection over members that may be trimmed.")]
5660
public static IReactiveBinding<TView, TProp> BindCommand<
5761
TView,
@@ -79,25 +83,29 @@ public static IReactiveBinding<TView, TProp> BindCommand<
7983
}
8084

8185
/// <summary>
82-
/// Binds a command from the view model to a control on the view, enabling the control to execute the specified
83-
/// command when triggered.
86+
/// Binds a command from the view model to a control on the view, enabling the control to execute the command with a
87+
/// specified parameter when an event is raised.
8488
/// </summary>
85-
/// <remarks>This method uses reflection to observe events and properties on the control and may be
86-
/// affected by trimming in environments that remove unused members. The binding enables the control to execute the
87-
/// command when the specified event is raised, and automatically manages the enabled state of the control based on
88-
/// the command's CanExecute state.</remarks>
89-
/// <typeparam name="TView">The type of the view implementing the IViewFor interface.</typeparam>
90-
/// <typeparam name="TViewModel">The type of the view model containing the command property.</typeparam>
91-
/// <typeparam name="TProp">The type of the command property to bind, implementing ICommand.</typeparam>
89+
/// <remarks>
90+
/// <para>The binding enables the control to execute the command when the specified event is raised,
91+
/// and automatically manages the enabled state of the control based on the command's CanExecute state.</para>
92+
/// <para>This method uses reflection to observe events and properties on the control, which may be
93+
/// affected by trimming in some deployment scenarios.</para>
94+
/// </remarks>
95+
/// <typeparam name="TView">The type of the view that implements the IViewFor interface.</typeparam>
96+
/// <typeparam name="TViewModel">The type of the view model containing the command to bind.</typeparam>
97+
/// <typeparam name="TProp">The type of the command property on the view model. Must implement ICommand.</typeparam>
9298
/// <typeparam name="TControl">The type of the control on the view to which the command will be bound.</typeparam>
93-
/// <param name="view">The view instance to which the control belongs. Cannot be null.</param>
94-
/// <param name="viewModel">The view model instance containing the command property. Can be null if the view's ViewModel property is used.</param>
99+
/// <param name="view">The view instance containing the control to which the command will be bound. Cannot be null.</param>
100+
/// <param name="viewModel">The view model instance containing the command to bind. Used for type inference.
101+
/// Can be null if the binding should be established without an initial view model.</param>
95102
/// <param name="propertyName">An expression identifying the command property on the view model to bind. Cannot be null.</param>
96-
/// <param name="controlName">An expression identifying the control on the view to bind the command to. Cannot be null.</param>
97-
/// <param name="toEvent">The name of the event on the control that triggers the command. If null, a default event is used based on the
98-
/// control type.
103+
/// <param name="controlName">An expression identifying the control on the view to which the command will be bound. Cannot be null.</param>
104+
/// <param name="toEvent">The name of the event on the control that triggers the command execution. If null, a default event is used based
105+
/// on the control type. If the specified event does not exist on the control, an exception may be thrown at runtime.
99106
/// NOTE: If this parameter is used inside WhenActivated, it's important to dispose the binding when the view is deactivated.</param>
100-
/// <returns>An object representing the binding between the command and the control, which can be disposed to unbind.</returns>
107+
/// <returns>An IReactiveBinding{TView, TProp} representing the established binding between the command and the control.
108+
/// It will remain active until disposed.</returns>
101109
[RequiresUnreferencedCode("Dynamic observation uses reflection over members that may be trimmed.")]
102110
public static IReactiveBinding<TView, TProp> BindCommand<
103111
TView,
@@ -123,26 +131,30 @@ public static IReactiveBinding<TView, TProp> BindCommand<
123131

124132
/// <summary>
125133
/// Binds a command from the view model to a control on the view, enabling the control to execute the command with a
126-
/// specified parameter when triggered.
134+
/// specified parameter when an event is raised.
127135
/// </summary>
128-
/// <remarks>This method uses reflection to observe events and properties on the control and view model,
129-
/// which may be affected by trimming in some deployment scenarios. The binding remains active until the returned
130-
/// IReactiveBinding is disposed.</remarks>
131-
/// <typeparam name="TView">The type of the view implementing the IViewFor interface.</typeparam>
132-
/// <typeparam name="TViewModel">The type of the view model containing the command property.</typeparam>
133-
/// <typeparam name="TProp">The type of the command property, typically implementing ICommand.</typeparam>
136+
/// <remarks>
137+
/// <para>The binding enables the control to execute the command when the specified event is raised,
138+
/// and automatically manages the enabled state of the control based on the command's CanExecute state.</para>
139+
/// <para>This method uses reflection to observe events and properties on the control, which may be
140+
/// affected by trimming in some deployment scenarios.</para>
141+
/// </remarks>
142+
/// <typeparam name="TView">The type of the view that implements the IViewFor interface.</typeparam>
143+
/// <typeparam name="TViewModel">The type of the view model containing the command to bind.</typeparam>
144+
/// <typeparam name="TProp">The type of the command property on the view model. Must implement ICommand.</typeparam>
134145
/// <typeparam name="TControl">The type of the control on the view to which the command will be bound.</typeparam>
135-
/// <typeparam name="TParam">The type of the parameter passed to the command when it is executed.</typeparam>
136-
/// <param name="view">The view instance containing the control to bind the command to. Cannot be null.</param>
137-
/// <param name="viewModel">The view model instance containing the command property. May be null if the view is not currently bound to a
138-
/// view model.</param>
146+
/// <typeparam name="TParam">The type of the parameter passed to the command when the event is raised.</typeparam>
147+
/// <param name="view">The view instance containing the control to which the command will be bound. Cannot be null.</param>
148+
/// <param name="viewModel">The view model instance containing the command to bind. Used for type inference.
149+
/// Can be null if the binding should be established without an initial view model.</param>
139150
/// <param name="propertyName">An expression identifying the command property on the view model to bind. Cannot be null.</param>
140151
/// <param name="controlName">An expression identifying the control on the view to which the command will be bound. Cannot be null.</param>
141152
/// <param name="withParameter">An expression specifying the parameter to pass to the command when it is executed. Cannot be null.</param>
142153
/// <param name="toEvent">The name of the event on the control that triggers the command execution. If null, a default event is used based
143-
/// on the control type.
154+
/// on the control type. If the specified event does not exist on the control, an exception may be thrown at runtime.
144155
/// NOTE: If this parameter is used inside WhenActivated, it's important to dispose the binding when the view is deactivated.</param>
145-
/// <returns>An IReactiveBinding{TView, TProp} representing the established binding between the command and the control.</returns>
156+
/// <returns>An IReactiveBinding{TView, TProp} representing the established binding between the command and the control.
157+
/// It will remain active until disposed.</returns>
146158
[RequiresUnreferencedCode("Dynamic observation uses reflection over members that may be trimmed.")]
147159
public static IReactiveBinding<TView, TProp> BindCommand<
148160
TView,

src/ReactiveUI/Bindings/Command/CommandBinderImplementation.cs

Lines changed: 9 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -25,29 +25,7 @@ namespace ReactiveUI;
2525
/// </remarks>
2626
public class CommandBinderImplementation : ICommandBinderImplementation
2727
{
28-
/// <summary>
29-
/// Binds a command from the view model to a control on the view, enabling the control to execute the command with
30-
/// an optional parameter when triggered by a specified event.
31-
/// </summary>
32-
/// <remarks>This method uses reflection to observe properties and events, which may be affected by
33-
/// trimming in some deployment scenarios. The binding is one-way, from the view model command to the view control.
34-
/// If the specified event is not found on the control, an exception may be thrown at runtime.</remarks>
35-
/// <typeparam name="TView">The type of the view implementing the IViewFor interface.</typeparam>
36-
/// <typeparam name="TViewModel">The type of the view model containing the command property.</typeparam>
37-
/// <typeparam name="TProp">The type of the command property to bind, implementing ICommand.</typeparam>
38-
/// <typeparam name="TControl">The type of the control on the view to which the command will be bound.</typeparam>
39-
/// <typeparam name="TParam">The type of the parameter passed to the command when it is executed.</typeparam>
40-
/// <param name="viewModel">The view model instance containing the command to bind. Can be null if the binding should be established without
41-
/// an initial view model.</param>
42-
/// <param name="view">The view instance containing the control to which the command will be bound. Cannot be null.</param>
43-
/// <param name="vmProperty">An expression specifying the command property on the view model to bind. Cannot be null.</param>
44-
/// <param name="controlProperty">An expression specifying the control on the view to which the command will be bound. Cannot be null.</param>
45-
/// <param name="withParameter">An expression specifying the parameter to pass to the command when it is executed. Can be null if the command
46-
/// does not require a parameter.</param>
47-
/// <param name="toEvent">The name of the event on the control that triggers the command execution. If null, a default event is used based
48-
/// on the control type.</param>
49-
/// <returns>An IReactiveBinding{TView, TProp} representing the established command binding. Disposing the returned object
50-
/// will remove the binding.</returns>
28+
/// <inheritdoc />
5129
[RequiresUnreferencedCode("Dynamic observation uses reflection over members that may be trimmed.")]
5230
public IReactiveBinding<TView, TProp> BindCommand<
5331
TView,
@@ -68,17 +46,20 @@ public IReactiveBinding<TView, TProp> BindCommand<
6846
{
6947
ArgumentExceptionHelper.ThrowIfNull(vmProperty);
7048
ArgumentExceptionHelper.ThrowIfNull(controlProperty);
49+
ArgumentExceptionHelper.ThrowIfNull(withParameter);
7150

7251
var vmExpression = Reflection.Rewrite(vmProperty.Body);
7352
var controlExpression = Reflection.Rewrite(controlProperty.Body);
53+
var parameterExpression = Reflection.Rewrite(withParameter.Body);
7454

7555
var source = Reflection.ViewModelWhenAnyValue(viewModel, view, vmExpression).Cast<TProp>();
56+
var parameterObservable = Reflection.ViewModelWhenAnyValue(viewModel, view, parameterExpression).Cast<TParam>();
7657

7758
var bindingDisposable = BindCommandInternal<TView, TProp, TParam, TControl>(
7859
source,
7960
view,
8061
controlExpression,
81-
withParameter.ToObservable(viewModel),
62+
parameterObservable,
8263
toEvent);
8364

8465
return new ReactiveBinding<TView, TProp>(
@@ -90,31 +71,7 @@ public IReactiveBinding<TView, TProp> BindCommand<
9071
bindingDisposable);
9172
}
9273

93-
/// <summary>
94-
/// Binds a command from the view model to a control on the view, enabling the control to execute the command with
95-
/// an optional parameter stream and event trigger.
96-
/// </summary>
97-
/// <remarks>This method uses reflection to observe and bind to members, which may be affected by trimming
98-
/// in some environments. The binding is one-way, from the view model command to the view control. If the control or
99-
/// command property is not found, the binding will not be established. The method is suitable for scenarios where
100-
/// commands need to be dynamically bound to controls with support for parameter streams and custom event
101-
/// triggers.</remarks>
102-
/// <typeparam name="TView">The type of the view implementing the IViewFor interface.</typeparam>
103-
/// <typeparam name="TViewModel">The type of the view model containing the command property.</typeparam>
104-
/// <typeparam name="TProp">The type of the command property, which must implement ICommand.</typeparam>
105-
/// <typeparam name="TControl">The type of the control on the view to which the command will be bound.</typeparam>
106-
/// <typeparam name="TParam">The type of the parameter passed to the command when it is executed.</typeparam>
107-
/// <param name="viewModel">The view model instance containing the command to bind. Can be null if the view model is not available at
108-
/// binding time.</param>
109-
/// <param name="view">The view instance containing the control to which the command will be bound.</param>
110-
/// <param name="vmProperty">An expression specifying the command property on the view model to bind.</param>
111-
/// <param name="controlProperty">An expression specifying the control on the view that will trigger the command.</param>
112-
/// <param name="withParameter">An observable sequence providing the parameter to pass to the command when it is executed. The latest value is
113-
/// used for each command invocation.</param>
114-
/// <param name="toEvent">The name of the event on the control that triggers the command. If null, a default event is used based on the
115-
/// control type.</param>
116-
/// <returns>An IReactiveBinding{TView, TProp} representing the established binding between the command and the control.
117-
/// Disposing the binding will remove the command association.</returns>
74+
/// <inheritdoc />
11875
[RequiresUnreferencedCode("Dynamic observation uses reflection over members that may be trimmed.")]
11976
public IReactiveBinding<TView, TProp> BindCommand<
12077
TView,
@@ -197,7 +154,9 @@ private static IDisposable BindCommandInternal<
197154
var isInitialBind = true;
198155

199156
// Check for optional platform-specific command rebinding customization
200-
var rebindingCustomizer = AppLocator.Current.GetService<ICreatesCustomizedCommandRebinding>();
157+
var rebindingCustomizer = string.IsNullOrEmpty(toEvent)
158+
? AppLocator.Current.GetService<ICreatesCustomizedCommandRebinding>()
159+
: null;
201160

202161
// Cache boxing of parameter values once to avoid rebuilding the Select pipeline on every rebind.
203162
var boxedParameter = withParameter.Select(static p => (object?)p);

0 commit comments

Comments
 (0)