Skip to content

Commit be0eb89

Browse files
ChrisPulmanCopilot
ChrisPulman
and
Copilot
authored
Fix nested property binding to avoid redundant setter calls (#4240)
<!-- Please be sure to read the [Contribute](https://github.com/reactiveui/reactiveui#contribute) section of the README --> **What kind of change does this PR introduce?** <!-- Bug fix, feature, docs update, ... --> Fixes #4214 **What is the current behavior?** <!-- You can also link to an open issue here. --> #4214 **What is the new behavior?** <!-- If this is a feature change --> Adds tests and refactors PropertyBinderImplementation to ensure that BindTo only sets nested properties once per value, even when the host or nested object is replaced. This prevents redundant setter invocations and ensures correct property update semantics for nested bindings across platforms. **What might this PR break?** One less value should be emitted from BindTo when used in the two-way binding manner as shown in #4214 **Please check if the PR fulfills these requirements** - [x] Tests for the changes have been added (for bug fixes / features) - [ ] Docs have been added / updated (for bug fixes / features) **Other information**: --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: ChrisPulman <[email protected]>
1 parent 3709a49 commit be0eb89

File tree

4 files changed

+378
-28
lines changed

4 files changed

+378
-28
lines changed

‎src/ReactiveUI.Tests/Bindings/BindToNestedPropertyTests.cs‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
// Copyright (c) 2025 .NET Foundation and Contributors. All rights reserved.
2+
// Licensed to the .NET Foundation under one or more agreements.
3+
// The .NET Foundation licenses this file to you under the MIT license.
4+
// See the LICENSE file in the project root for full license information.
5+
6+
namespace ReactiveUI.Tests.Bindings;
7+
8+
[TestFixture]
9+
public class BindToNestedPropertyTests
10+
{
11+
[Test]
12+
public void BindToSetsNestedPropertyOncePerValueOnSameHost()
13+
{
14+
var view = new TextTrackingView();
15+
16+
using var subscription1 = view.WhenAnyValue(x => x.ViewModel!.Nested.SomeText).BindTo(view, static x => x.TextField);
17+
using var subscription2 = view.WhenAnyValue(x => x.TextField).BindTo(view, static x => x.ViewModel!.Nested.SomeText);
18+
19+
view.ViewModel.Nested = new() { SomeText = "Alpha" };
20+
view.ViewModel.Nested = new() { SomeText = "Alpha" };
21+
view.ViewModel.Nested = new() { SomeText = "Alpha" };
22+
view.ViewModel.Nested = new() { SomeText = "Beta" };
23+
view.ViewModel.Nested = new() { SomeText = "Beta" };
24+
view.ViewModel.Nested = new() { SomeText = "Beta" };
25+
view.ViewModel.Nested = new() { SomeText = "Gamma" };
26+
view.ViewModel.Nested = new() { SomeText = "Gamma" };
27+
view.ViewModel.Nested = new() { SomeText = "Gamma" };
28+
29+
var nested = view.ViewModel!.Nested;
30+
31+
using (Assert.EnterMultipleScope())
32+
{
33+
Assert.That(nested.SetCallCount, Is.EqualTo(1));
34+
Assert.That(view.SetCallCount, Is.EqualTo(3));
35+
Assert.That(nested.SomeText, Is.EqualTo("Gamma"));
36+
}
37+
}
38+
39+
[Test]
40+
public void BindToSetsNestedPropertyOncePerValueAfterHostReplacement()
41+
{
42+
var view = new TextTrackingView();
43+
44+
using var source = new Subject<string>();
45+
using var subscription = source.BindTo(view, static x => x.ViewModel.Nested.SomeText);
46+
47+
var values = new[] { "Delta", "Epsilon", "Zeta" };
48+
var count = 0;
49+
foreach (var value in values)
50+
{
51+
var replacement = new TrackingNestedValue();
52+
view.ViewModel.Nested = replacement;
53+
54+
source.OnNext(value);
55+
count++;
56+
57+
using (Assert.EnterMultipleScope())
58+
{
59+
Assert.That(replacement.SetCallCount, Is.EqualTo(1), $"Value '{value}' invoked the setter more than once.");
60+
Assert.That(replacement.SomeText, Is.EqualTo(value));
61+
}
62+
}
63+
}
64+
65+
[Test]
66+
public void BindToFromViewPropertyKeepsSingleSetterAfterHostReplacement()
67+
{
68+
var view = new TextTrackingView { TextField = "Initial" }; // 1
69+
70+
using var subscription = view.WhenAnyValue(x => x.TextField)
71+
.BindTo(view, static x => x.ViewModel!.Nested.SomeText);
72+
73+
var first = view.ViewModel.Nested;
74+
75+
view.TextField = "First"; // 2
76+
77+
using (Assert.EnterMultipleScope())
78+
{
79+
Assert.That(first.SetCallCount, Is.EqualTo(2));
80+
Assert.That(first.SomeText, Is.EqualTo("First"));
81+
}
82+
83+
var replacement = new TrackingNestedValue();
84+
view.ViewModel.Nested = replacement;
85+
86+
view.TextField = "Second"; // 3
87+
88+
using (Assert.EnterMultipleScope())
89+
{
90+
Assert.That(replacement.SetCallCount, Is.EqualTo(1), "Setter invoked multiple times after host swap.");
91+
Assert.That(replacement.SomeText, Is.EqualTo("Second"));
92+
}
93+
}
94+
95+
private sealed class TrackingHostViewModel : ReactiveObject
96+
{
97+
public TrackingHostViewModel() => Nested = new();
98+
99+
public TrackingNestedValue Nested
100+
{
101+
get => field;
102+
set => this.RaiseAndSetIfChanged(ref field, value);
103+
}
104+
}
105+
106+
private sealed class TrackingNestedValue : ReactiveObject
107+
{
108+
public int SetCallCount { get; private set; }
109+
110+
public string? SomeText
111+
{
112+
get => field;
113+
set
114+
{
115+
if (value != field)
116+
{
117+
this.RaisePropertyChanging();
118+
field = value;
119+
this.RaisePropertyChanged();
120+
SetCallCount++;
121+
}
122+
}
123+
}
124+
}
125+
126+
private sealed class TextTrackingView : ReactiveObject
127+
{
128+
private TrackingHostViewModel _viewModel = new();
129+
130+
public int SetCallCount { get; private set; }
131+
132+
public TrackingHostViewModel ViewModel
133+
{
134+
get => _viewModel;
135+
set => this.RaiseAndSetIfChanged(ref _viewModel, value);
136+
}
137+
138+
public string? TextField
139+
{
140+
get => field;
141+
set
142+
{
143+
if (value != field)
144+
{
145+
this.RaisePropertyChanging();
146+
field = value;
147+
this.RaisePropertyChanged();
148+
SetCallCount++;
149+
}
150+
}
151+
}
152+
}
153+
}

‎src/ReactiveUI.Tests/Platforms/windows-xaml/PropertyBindingTest.cs‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections;
77
using System.Globalization;
88
using DynamicData.Binding;
9+
using ReactiveUI;
910

1011
namespace ReactiveUI.Tests.Xaml;
1112

@@ -1673,4 +1674,110 @@ public void BindWithFuncToTriggerUpdateTestViewModelToViewWithLongConverterNoHin
16731674
dis.Dispose();
16741675
Assert.That(dis.IsDisposed, Is.True);
16751676
}
1677+
1678+
/// <summary>
1679+
/// BindTo should only invoke the nested setter once per source value on the same host.
1680+
/// </summary>
1681+
[Test]
1682+
public void BindToSetsNestedPropertyOncePerValueOnSameHost()
1683+
{
1684+
var view = new TrackingHostView { ViewModel = new() };
1685+
1686+
using var source = new Subject<string>();
1687+
using var subscription = source.BindTo(view, static x => x.ViewModel!.Nested.SomeText);
1688+
1689+
source.OnNext("Alpha");
1690+
source.OnNext("Alpha");
1691+
source.OnNext("Alpha");
1692+
source.OnNext("Beta");
1693+
source.OnNext("Beta");
1694+
source.OnNext("Beta");
1695+
source.OnNext("Gamma");
1696+
source.OnNext("Gamma");
1697+
source.OnNext("Gamma");
1698+
1699+
var nested = view.ViewModel!.Nested;
1700+
1701+
Assert.Multiple(() =>
1702+
{
1703+
Assert.That(nested.SetCallCount, Is.EqualTo(3));
1704+
Assert.That(nested.SomeText, Is.EqualTo("Gamma"));
1705+
});
1706+
}
1707+
1708+
/// <summary>
1709+
/// BindTo should not reapply stale values after replacing the nested host.
1710+
/// </summary>
1711+
[Test]
1712+
public void BindToSetsNestedPropertyOncePerValueAfterHostReplacement()
1713+
{
1714+
var view = new TrackingHostView { ViewModel = new() };
1715+
1716+
using var source = new Subject<string>();
1717+
using var subscription = source.BindTo(view, static x => x.ViewModel!.Nested.SomeText);
1718+
1719+
var values = new[] { "Delta", "Epsilon", "Zeta" };
1720+
1721+
foreach (var value in values)
1722+
{
1723+
var replacement = new TrackingNestedValue();
1724+
view.ViewModel!.Nested = replacement;
1725+
1726+
source.OnNext(value);
1727+
1728+
Assert.Multiple(() =>
1729+
{
1730+
Assert.That(replacement.SetCallCount, Is.EqualTo(1), $"Value '{value}' invoked the setter more than once.");
1731+
Assert.That(replacement.SomeText, Is.EqualTo(value));
1732+
});
1733+
}
1734+
}
1735+
1736+
private sealed class TrackingHostView : ReactiveObject, IViewFor<TrackingHostViewModel>
1737+
{
1738+
private TrackingHostViewModel? _viewModel;
1739+
1740+
public TrackingHostViewModel? ViewModel
1741+
{
1742+
get => _viewModel;
1743+
set => this.RaiseAndSetIfChanged(ref _viewModel, value);
1744+
}
1745+
1746+
object? IViewFor.ViewModel
1747+
{
1748+
get => ViewModel;
1749+
set => ViewModel = (TrackingHostViewModel?)value;
1750+
}
1751+
}
1752+
1753+
private sealed class TrackingHostViewModel : ReactiveObject
1754+
{
1755+
private TrackingNestedValue _nested = new();
1756+
1757+
public TrackingNestedValue Nested
1758+
{
1759+
get => _nested;
1760+
set => this.RaiseAndSetIfChanged(ref _nested, value);
1761+
}
1762+
}
1763+
1764+
private sealed class TrackingNestedValue : ReactiveObject
1765+
{
1766+
public int SetCallCount { get; private set; }
1767+
1768+
public string? SomeText
1769+
{
1770+
get => field;
1771+
set
1772+
{
1773+
if (value != field)
1774+
{
1775+
this.RaisePropertyChanging();
1776+
field = value;
1777+
this.RaisePropertyChanged();
1778+
SetCallCount++;
1779+
}
1780+
}
1781+
}
1782+
}
16761783
}

‎src/ReactiveUI.Tests/Platforms/winforms/DefaultPropertyBindingTests.cs‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@
55

66
using System.Globalization;
77
using System.Windows.Forms;
8-
98
using DynamicData;
10-
11-
using ReactiveUI.Winforms;
129
using ReactiveUI.Tests.Infrastructure.StaticState;
10+
using ReactiveUI.Winforms;
1311

1412
namespace ReactiveUI.Tests.Winforms;
1513

@@ -30,22 +28,13 @@ public class DefaultPropertyBindingTests
3028
/// <summary>
3129
/// Initializes a new instance of the <see cref="DefaultPropertyBindingTests"/> class.
3230
/// </summary>
33-
public DefaultPropertyBindingTests()
34-
{
35-
RxApp.EnsureInitialized();
36-
}
31+
public DefaultPropertyBindingTests() => RxApp.EnsureInitialized();
3732

3833
[SetUp]
39-
public void SetUp()
40-
{
41-
_schedulersScope = new RxAppSchedulersScope();
42-
}
34+
public void SetUp() => _schedulersScope = new RxAppSchedulersScope();
4335

4436
[TearDown]
45-
public void TearDown()
46-
{
47-
_schedulersScope?.Dispose();
48-
}
37+
public void TearDown() => _schedulersScope?.Dispose();
4938

5039
/// <summary>
5140
/// Tests Winforms creates observable for property works for textboxes.

0 commit comments

Comments
 (0)