From e1c9765a0b7721cfe12358f10b7a563a55ad4541 Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Fri, 27 Feb 2026 21:21:07 +0100 Subject: [PATCH] Fix TypedBinding nested property re-subscription after null intermediate becomes non-null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an intermediate object in a TypedBinding path starts as null and later becomes non-null (or is replaced with a different object), the binding must re-establish INPC subscriptions to the new object's nested properties. The previous optimization used a '_isSubscribed' boolean flag to skip IPropertyChangeHandler.Subscribe() after the first Apply. This prevented re-subscribing when intermediate objects changed. For example: vm.Child = null // subscribe to vm, but NOT vm.Child.Name (null) vm.Child = new Child() // Subscribe skipped due to _isSubscribed=true vm.Child.Name = "x" // binding never fires — regression! The fix removes '_isSubscribed' and always calls Subscribe on each Apply. Both IPropertyChangeHandler implementations (PropertyChangeHandler and LegacyPropertyChangeHandler) are already idempotent — they use reference equality checks to avoid re-subscribing to unchanged objects — so calling Subscribe on every Apply is safe and has minimal overhead. Adds two regression tests: - TypedBinding_NestedProperty_ResubscribesAfterNullIntermediateBecomesNonNull - TypedBinding_NestedProperty_ResubscribesAfterIntermediateReplaced Reported in: https://github.com/dotnet/maui/pull/32382#issuecomment-3953258371 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Controls/src/Core/TypedBinding.cs | 9 +-- .../Core.UnitTests/TypedBindingUnitTests.cs | 81 +++++++++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/Controls/src/Core/TypedBinding.cs b/src/Controls/src/Core/TypedBinding.cs index 90b1a1d9f30e..4a5ad5545fb2 100644 --- a/src/Controls/src/Core/TypedBinding.cs +++ b/src/Controls/src/Core/TypedBinding.cs @@ -147,7 +147,6 @@ public TypedBinding(Func getter, Actio List> _ancestryChain; bool _isBindingContextRelativeSource; BindingMode _cachedMode; - bool _isSubscribed; bool _isTSource; // cached type check result object _cachedDefaultValue; // cached default value bool _hasDefaultValue; @@ -289,7 +288,6 @@ internal override void Unapply(bool fromBindingContextChanged = false) if (_handlers != null) Unsubscribe(); - _isSubscribed = false; _cachedMode = BindingMode.Default; _hasDefaultValue = false; _cachedDefaultValue = null; @@ -332,11 +330,12 @@ internal void ApplyCore(object sourceObject, BindableObject target, BindableProp var needsGetter = (mode == BindingMode.TwoWay && !fromTarget) || mode == BindingMode.OneWay || mode == BindingMode.OneTime; - // Only subscribe once per binding lifetime - if (!_isSubscribed && isTSource && (mode == BindingMode.OneWay || mode == BindingMode.TwoWay) && _handlers != null) + // Subscribe on every Apply so that intermediate objects that changed are re-subscribed. + // Subscribe() is idempotent: it diffs old vs new subscription targets and only + // updates what changed, so calling this repeatedly is safe. + if (isTSource && (mode == BindingMode.OneWay || mode == BindingMode.TwoWay) && _handlers != null) { Subscribe((TSource)sourceObject); - _isSubscribed = true; } if (needsGetter) diff --git a/src/Controls/tests/Core.UnitTests/TypedBindingUnitTests.cs b/src/Controls/tests/Core.UnitTests/TypedBindingUnitTests.cs index 5fbf2f11f096..e8ceee33570a 100644 --- a/src/Controls/tests/Core.UnitTests/TypedBindingUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/TypedBindingUnitTests.cs @@ -1773,6 +1773,87 @@ public string Title } } + [Fact] + //https://github.com/dotnet/maui/issues/34428 + public void TypedBinding_NestedProperty_ResubscribesAfterNullIntermediateBecomesNonNull() + { + // Regression: when an intermediate object in the path starts as null and later becomes + // non-null, the binding must re-establish subscriptions to nested properties. + // Previously, the _isSubscribed flag prevented re-subscribing after the first Apply. + + var vm = new ComplexMockViewModel + { + Model = null // Start with null intermediate + }; + + var property = BindableProperty.Create("Text", typeof(string), typeof(MockBindable), null); + + var binding = new TypedBinding( + cvm => cvm.Model is { } m ? (m.Text, true) : (null, false), + (cvm, t) => { if (cvm.Model is { } m) m.Text = t; }, + new[] { + new Tuple, string>(cvm => cvm, "Model"), + new Tuple, string>(cvm => cvm.Model, "Text") + }) + { Mode = BindingMode.OneWay }; + + var bindable = new MockBindable(); + bindable.SetBinding(property, binding); + bindable.BindingContext = vm; + + // Initially null model → binding returns null/default + Assert.Null(bindable.GetValue(property)); + + // Set Model to non-null → binding should pick up the value + vm.Model = new ComplexMockViewModel { Text = "Initial" }; + Assert.Equal("Initial", (string)bindable.GetValue(property)); + + // Change nested property → binding MUST update (this was the regression) + vm.Model.Text = "Updated"; + Assert.Equal("Updated", (string)bindable.GetValue(property)); + } + + [Fact] + //https://github.com/dotnet/maui/issues/34428 + public void TypedBinding_NestedProperty_ResubscribesAfterIntermediateReplaced() + { + // When the intermediate object is replaced (non-null → different non-null object), + // the binding must switch subscriptions to the new object. + + var child1 = new ComplexMockViewModel { Text = "Child1" }; + var child2 = new ComplexMockViewModel { Text = "Child2" }; + var vm = new ComplexMockViewModel { Model = child1 }; + + var property = BindableProperty.Create("Text", typeof(string), typeof(MockBindable), null); + + var binding = new TypedBinding( + cvm => cvm.Model is { } m ? (m.Text, true) : (null, false), + (cvm, t) => { if (cvm.Model is { } m) m.Text = t; }, + new[] { + new Tuple, string>(cvm => cvm, "Model"), + new Tuple, string>(cvm => cvm.Model, "Text") + }) + { Mode = BindingMode.OneWay }; + + var bindable = new MockBindable(); + bindable.SetBinding(property, binding); + bindable.BindingContext = vm; + + Assert.Equal("Child1", (string)bindable.GetValue(property)); + + // Replace intermediate with a different object + vm.Model = child2; + Assert.Equal("Child2", (string)bindable.GetValue(property)); + + // Changing the OLD intermediate should NOT fire the binding + child1.Text = "OldChildChanged"; + Assert.Equal("Child2", (string)bindable.GetValue(property)); + + // Changing the NEW intermediate SHOULD fire the binding + child2.Text = "Child2Updated"; + Assert.Equal("Child2Updated", (string)bindable.GetValue(property)); + } + [Fact] //https://github.com/xamarin/Microsoft.Maui.Controls/issues/3650 //https://github.com/xamarin/Microsoft.Maui.Controls/issues/3613