BindingContext changes are called multiple times on app start #470

adrianknight89 · 2016-10-19T06:19:07Z

Description of Change

If you put a view model locator as an App resource and set your binding context in MainPage, the locator gets called multiple times. This happens first when XAML for the main page is being loaded and then when the page is being parented by App.

This PR ensures that binding context changes are not made if the inherited context is the same as the value passed to SetInheritedBindingContext()

Bugs Fixed

PR Checklist

Has tests (if omitted, state reason in description)
Rebased on top of master at time of PR
Changes adhere to coding standard
Consolidate commits as makes sense

StephaneDelcroix

This actually hides a part of the issue*, but I'm ok to go with it, as it fixes the issue at hand, provided you add a unittest for it.

[*] full story here:

Bindings are applied on SetInheritedBindingContext even when the context does not change => you fixed this
Bindings are applied twice on Parenting; once in set_Parent and a second time in OnChildAdded. => I'm fine if we do not fix this
This is not a bug, merely an improvement. We should be able to poke the binding source as often as we find convenient :)

adrianknight89 · 2016-10-19T07:35:20Z

Xamarin.Forms.Core/Element.cs

 			child.Parent = this;
 			if (Platform != null)
 				child.Platform = Platform;

-			child.ApplyBindings();
+			if(bindingContext == child.BindingContext)
+				child.ApplyBindings();


In general, setting Parent should set child binding context, but there are two checks in parent set that return before SetChildInheritedBindingContext() is called. It seemed safer to add this check instead of removing apply bindings altogether. I'll work on unit testing tomorrow.

it's actually safer to leave this part of the code unchanged, and stay with the safe first version of the PR

This reverts commit 71952f3.

rolek · 2017-05-03T16:42:29Z

This still happens in v 2.3.4.231

Szymaniuk · 2017-06-13T11:39:17Z

This is merged with what? With master, that's fine, but with which Xamarin.Forms release, because for me it's also not working at 2.3.4.x.

StephaneDelcroix · 2017-06-13T13:32:41Z

@Szymaniuk this was released in the 2.3.4 series.

With this PR, and e.g. #983, we're reducing the number of times the Binding gets its source. We still do not consider it a bug. See #470 (review) for explanation.

If you've a case when the Binding is updated when you think it shouldn't, please report a bug to bugzilla, we'll see if this is fixable or not.

j-fritsch · 2017-06-13T15:29:56Z

@StephaneDelcroix, thanks for the update. In my case, after updating to 2.3.4, the number of times my ViewModelLocator was called dropped from 3 to 2. However, I think I still need a little more explanation. Sorry if I didn't fully understand your review above.

As described in the bugzilla reports (1) (2), if I set the BindingContext in XAML, my ViewModelLocator will be called multiple times.

<ContentPage
  BindingContext="{Binding Home, Source={StaticResource ViewModelLocator}}">

If I set the BindingContext in the page's constructor in the codebehind, my ViewModelLocator is only called once.

public HomePage()
{
    InitializeComponent();

    var vml = (ViewModelLocator)Application.Current.Resources["ViewModelLocator"];
    BindingContext = vml.Home;
}

Are you saying that is normal, expected behavior? If I want or need it to only be called once, the solution is to just set it in the page's constructor instead?

StephaneDelcroix · 2017-06-14T06:41:51Z

@j-fritsch this isn't a wrong behavior, but #983 is supposed to address this anyway.

inherited binding context fix

5c94392

dnfclas added the cla-already-signed label Oct 19, 2016

StephaneDelcroix suggested changes Oct 19, 2016

View reviewed changes

add check for child binding context

71952f3

adrianknight89 commented Oct 19, 2016

View reviewed changes

adrianknight89 added 2 commits October 19, 2016 03:32

Revert "add check for child binding context"

c626f27

This reverts commit 71952f3.

unit tests

d401ac8

StephaneDelcroix self-assigned this Oct 20, 2016

StephaneDelcroix merged commit 107ed5e into xamarin:master Nov 16, 2016

davidortinau mentioned this pull request Dec 20, 2017

Viewmodel instantiates twice using a ViewModelLocator #1434

Closed

samhouts added this to the 2.5.1 milestone May 5, 2018

samhouts modified the milestones: 2.5.1, 2.3.0, 2.3.4 Jun 27, 2018

samhouts added the hacktoberfest 🍻 label Oct 1, 2019

samhouts added the t/bug 🐛 label Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BindingContext changes are called multiple times on app start #470

BindingContext changes are called multiple times on app start #470

adrianknight89 commented Oct 19, 2016 •

edited by samhouts

Loading

StephaneDelcroix left a comment •

edited

Loading

adrianknight89 Oct 19, 2016

StephaneDelcroix Oct 19, 2016

rolek commented May 3, 2017

Szymaniuk commented Jun 13, 2017 •

edited

Loading

StephaneDelcroix commented Jun 13, 2017 •

edited

Loading

j-fritsch commented Jun 13, 2017

StephaneDelcroix commented Jun 14, 2017

BindingContext changes are called multiple times on app start #470

BindingContext changes are called multiple times on app start #470

Conversation

adrianknight89 commented Oct 19, 2016 • edited by samhouts Loading

Description of Change

Bugs Fixed

PR Checklist

StephaneDelcroix left a comment • edited Loading

Choose a reason for hiding this comment

adrianknight89 Oct 19, 2016

Choose a reason for hiding this comment

StephaneDelcroix Oct 19, 2016

Choose a reason for hiding this comment

rolek commented May 3, 2017

Szymaniuk commented Jun 13, 2017 • edited Loading

StephaneDelcroix commented Jun 13, 2017 • edited Loading

j-fritsch commented Jun 13, 2017

StephaneDelcroix commented Jun 14, 2017

adrianknight89 commented Oct 19, 2016 •

edited by samhouts

Loading

StephaneDelcroix left a comment •

edited

Loading

Szymaniuk commented Jun 13, 2017 •

edited

Loading

StephaneDelcroix commented Jun 13, 2017 •

edited

Loading