From 95fbd1c5e906c1fa2859fd8aaf378a05fcb6f12f Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:17:28 +1000 Subject: [PATCH] fix: dispose the displaced binding when BindCommandUnsafe rebinds - RuntimeCommandBindingFallback and RuntimeInteractionFallback hold the current binding in a SwapDisposable, so a rebind disposes the binding it replaces. - Add tests for one active binding per call and for disposal after a rebind. - Build the .NET 11 assemblies without runtime-async. Mono does not support it, so it breaks Blazor WebAssembly and other Mono hosts. - Update TUnit to 1.72.16. - Fix the benchmarks for ReactiveUI 25: drop an unused using, and let the generated ToProperty name CountViewModel. Fixes #163 Co-Authored-By: Claude Opus 5.5 --- src/Directory.Build.props | 4 - src/Directory.Packages.props | 2 +- .../Fallback/RuntimeCommandBindingFallback.cs | 2 +- .../Fallback/RuntimeInteractionFallback.cs | 2 +- .../ReactiveUIToPropertyBenchmark.cs | 4 +- .../RxUiDynamicChainBaseline.cs | 1 - .../RuntimeCommandBindingFallbackTests.cs | 88 +++++++++++++++++++ .../RuntimeInteractionFallbackTests.cs | 58 ++++++++++++ .../TestModels/RecordingStubCommand.cs | 9 +- 9 files changed, 159 insertions(+), 11 deletions(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index fd8d441f..01b70b16 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -131,10 +131,6 @@ - - $(Features);runtime-async=on - - diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 1774e900..104cfb8b 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -16,7 +16,7 @@ - + diff --git a/src/ReactiveUI.Binding.Shared/Fallback/RuntimeCommandBindingFallback.cs b/src/ReactiveUI.Binding.Shared/Fallback/RuntimeCommandBindingFallback.cs index 67dfa2b6..ac951f90 100644 --- a/src/ReactiveUI.Binding.Shared/Fallback/RuntimeCommandBindingFallback.cs +++ b/src/ReactiveUI.Binding.Shared/Fallback/RuntimeCommandBindingFallback.cs @@ -61,7 +61,7 @@ public static IDisposable BindCommand< return EmptyDisposable.Instance; } - var binding = new MutableDisposable(); + var binding = new SwapDisposable(); var commands = RuntimeObservationFallback.WhenAnyValue(viewModel, commandProperty); var controls = RuntimeObservationFallback.WhenAnyValue(view, controlProperty); diff --git a/src/ReactiveUI.Binding.Shared/Fallback/RuntimeInteractionFallback.cs b/src/ReactiveUI.Binding.Shared/Fallback/RuntimeInteractionFallback.cs index 690f136b..87307314 100644 --- a/src/ReactiveUI.Binding.Shared/Fallback/RuntimeInteractionFallback.cs +++ b/src/ReactiveUI.Binding.Shared/Fallback/RuntimeInteractionFallback.cs @@ -50,7 +50,7 @@ public static IDisposable BindInteraction( return EmptyDisposable.Instance; } - var registration = new MutableDisposable(); + var registration = new SwapDisposable(); var observation = BindingErrors.Subscribe( RuntimeObservationFallback.WhenAnyValue(viewModel, interactionProperty), interaction => registration.Disposable = interaction is null diff --git a/src/benchmarks/ReactiveUI.Binding.Benchmarks.ReactiveUI/ReactiveUIToPropertyBenchmark.cs b/src/benchmarks/ReactiveUI.Binding.Benchmarks.ReactiveUI/ReactiveUIToPropertyBenchmark.cs index d7ba0312..8ccfcee4 100644 --- a/src/benchmarks/ReactiveUI.Binding.Benchmarks.ReactiveUI/ReactiveUIToPropertyBenchmark.cs +++ b/src/benchmarks/ReactiveUI.Binding.Benchmarks.ReactiveUI/ReactiveUIToPropertyBenchmark.cs @@ -102,14 +102,14 @@ protected virtual void Dispose(bool disposing) private void OnPropertyChanged(object? sender, PropertyChangedEventArgs args) => _notifications++; /// A ReactiveObject whose derived property is backed by ReactiveUI's helper. - private sealed class CountViewModel : ReactiveObject, IDisposable + internal sealed class CountViewModel : ReactiveObject, IDisposable { /// Backs . private readonly ObservableAsPropertyHelper _count; /// Initializes a new instance of the class. /// The values takes. - public CountViewModel(IObservable counts) => _count = counts.ToProperty(this, x => x.Count); + public CountViewModel(IObservable counts) => _count = counts.ToProperty(this, static x => x.Count); /// Gets the latest count. public int Count => _count.Value; diff --git a/src/benchmarks/ReactiveUI.Binding.Benchmarks/RxUiDynamicChainBaseline.cs b/src/benchmarks/ReactiveUI.Binding.Benchmarks/RxUiDynamicChainBaseline.cs index 619a16b9..33c69109 100644 --- a/src/benchmarks/ReactiveUI.Binding.Benchmarks/RxUiDynamicChainBaseline.cs +++ b/src/benchmarks/ReactiveUI.Binding.Benchmarks/RxUiDynamicChainBaseline.cs @@ -8,7 +8,6 @@ using System.Linq.Expressions; using System.Runtime.CompilerServices; using BenchmarkDotNet.Attributes; -using ReactiveUI; using ReactiveUI.Builder; using BenchmarkVm = ReactiveUI.Binding.Benchmarks.Mocks.BenchmarkViewModel; diff --git a/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeCommandBindingFallbackTests.cs b/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeCommandBindingFallbackTests.cs index 4968ab70..48869ece 100644 --- a/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeCommandBindingFallbackTests.cs +++ b/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeCommandBindingFallbackTests.cs @@ -2,6 +2,8 @@ // ReactiveUI and Contributors licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. +using System.ComponentModel; +using System.Runtime.CompilerServices; using System.Windows.Input; using ReactiveUI.Binding.Fallback; using ReactiveUI.Binding.Tests.TestModels; @@ -90,4 +92,90 @@ public async Task BindCommand_WithANamedEventAndAControlNoBinderReaches_BindsNot await Assert.That(command.LastParameter).IsNull(); } + + /// A change notification on a link of the control chain rebinds without leaving the old binding attached. + /// A task representing the asynchronous test operation. + [Test] + public async Task BindCommand_WhenAChainLinkRaisesChanged_KeepsOneActiveBinding() + { + RuntimeObservationFallbackTests.EnsureInitialized(); + Locator.CurrentMutable.RegisterConstant(new ClickCommandBinder()); + var command = new RecordingStubCommand(); + var viewModel = new DispatchStubViewModel { Run = command }; + var view = new ChainedView(); + + using var binding = RuntimeCommandBindingFallback.BindCommand( + view, + viewModel, + x => x.Run, + x => x.Editor.Properties.Button, + new ManualObservable(), + null, + BindingExpression); + + view.Editor.RaisePropertiesChanged(); + view.Editor.RaisePropertiesChanged(); + view.Editor.Properties.Button.PerformClick(); + + await Assert.That(command.ExecuteCount).IsEqualTo(1); + } + + /// Disposing the binding detaches the command even after a link rebound it. + /// A task representing the asynchronous test operation. + [Test] + public async Task BindCommand_WhenDisposedAfterARebind_DetachesTheCommand() + { + RuntimeObservationFallbackTests.EnsureInitialized(); + Locator.CurrentMutable.RegisterConstant(new ClickCommandBinder()); + var command = new RecordingStubCommand(); + var viewModel = new DispatchStubViewModel { Run = command }; + var view = new ChainedView(); + + var binding = RuntimeCommandBindingFallback.BindCommand( + view, + viewModel, + x => x.Run, + x => x.Editor.Properties.Button, + new ManualObservable(), + null, + BindingExpression); + + view.Editor.RaisePropertiesChanged(); + view.Editor.RaisePropertiesChanged(); + binding.Dispose(); + view.Editor.Properties.Button.PerformClick(); + + await Assert.That(command.ExecuteCount).IsEqualTo(0); + } + + /// A view whose control sits behind a notifying chain. + private sealed class ChainedView : IViewFor + { + /// + public object? ViewModel { get; set; } + + /// Gets the editor holding the control. + public ChainedEditor Editor { get; } = new(); + } + + /// An editor that reports its properties changed without replacing them. + private sealed class ChainedEditor : INotifyPropertyChanged + { + /// + public event PropertyChangedEventHandler? PropertyChanged; + + /// Gets the properties holding the control. + public ChainedProperties Properties { get; } = new(); + + /// Raises a change notification for . + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void RaisePropertiesChanged() => PropertyChanged?.Invoke(this, new(nameof(Properties))); + } + + /// Holds the control a command binds to. + private sealed class ChainedProperties + { + /// Gets the control. + public DispatchStubControl Button { get; } = new(); + } } diff --git a/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeInteractionFallbackTests.cs b/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeInteractionFallbackTests.cs index 1f7b934f..1bf7efcc 100644 --- a/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeInteractionFallbackTests.cs +++ b/src/tests/ReactiveUI.Binding.Tests/Fallback/RuntimeInteractionFallbackTests.cs @@ -2,6 +2,7 @@ // ReactiveUI and Contributors licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. +using System.ComponentModel; using ReactiveUI.Binding.Fallback; using ReactiveUI.Binding.Tests.TestModels; @@ -13,6 +14,9 @@ public class RuntimeInteractionFallbackTests /// The expression text reported when the observation faults. private const string BindingExpression = "x => x.Confirm"; + /// The registrations made after the interaction is replaced once: the original and the replacement. + private const int RegistrationsAfterReplacement = 2; + /// A null view model holds no interaction, so the handler is never registered. /// A task representing the asynchronous test operation. [Test] @@ -56,6 +60,60 @@ public async Task BindInteraction_WhenThePropertyHoldsNoInteraction_RegistersNot await Assert.That(registrations).IsEqualTo(0); } + /// Replacing the interaction disposes the registration made on the one it displaced. + /// A task representing the asynchronous test operation. + [Test] + public async Task BindInteraction_WhenTheInteractionIsReplaced_DisposesTheDisplacedRegistration() + { + RuntimeObservationFallbackTests.EnsureInitialized(); + List handlers = []; + var viewModel = new NotifyingViewModel { Confirm = new Interaction() }; + + using var binding = RuntimeInteractionFallback.BindInteraction( + viewModel, + x => x.Confirm, + _ => + { + var handler = new TrackedHandler(); + handlers.Add(handler); + return handler; + }, + BindingExpression); + + viewModel.Confirm = new Interaction(); + + await Assert.That(handlers.Count).IsEqualTo(RegistrationsAfterReplacement); + await Assert.That(handlers[0].Disposals).IsEqualTo(1); + } + + /// A view model that raises a change notification when its interaction is replaced. + private sealed class NotifyingViewModel : INotifyPropertyChanged + { + /// + public event PropertyChangedEventHandler? PropertyChanged; + + /// Gets or sets the interaction a binding names. + public IInteraction Confirm + { + get => field; + set + { + field = value; + PropertyChanged?.Invoke(this, new(nameof(Confirm))); + } + } = null!; + } + + /// Counts how often it is disposed. + private sealed class TrackedHandler : IDisposable + { + /// Gets how many times the handler has been disposed. + public int Disposals { get; private set; } + + /// + public void Dispose() => Disposals++; + } + /// Stands in for the registration a handler would hand back. private sealed class UnregisteredHandler : IDisposable { diff --git a/src/tests/ReactiveUI.Binding.Tests/TestModels/RecordingStubCommand.cs b/src/tests/ReactiveUI.Binding.Tests/TestModels/RecordingStubCommand.cs index 34ac06a7..30639d93 100644 --- a/src/tests/ReactiveUI.Binding.Tests/TestModels/RecordingStubCommand.cs +++ b/src/tests/ReactiveUI.Binding.Tests/TestModels/RecordingStubCommand.cs @@ -21,11 +21,18 @@ public event EventHandler? CanExecuteChanged /// Gets the parameter the most recent execution carried. public object? LastParameter { get; private set; } + /// Gets how many times the command has executed. + public int ExecuteCount { get; private set; } + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool CanExecute(object? parameter) => true; /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Execute(object? parameter) => LastParameter = parameter; + public void Execute(object? parameter) + { + ExecuteCount++; + LastParameter = parameter; + } }