Bug 57081 - Viewmodel instantiates twice using a ViewModelLocator
Summary: Viewmodel instantiates twice using a ViewModelLocator
Status: IN_PROGRESS
Alias: None
Product: Forms
Classification: Xamarin
Component: Forms (show other bugs)
Version: 2.3.4
Hardware: PC Windows
: Highest normal
Target Milestone: ---
Assignee: Stephane Delcroix
URL:
: 45557 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-06-01 21:38 UTC by Sid Dubey
Modified: 2017-10-05 18:17 UTC (History)
11 users (show)

See Also:
Tags: ac ios android uwp bindings navigation fr
Is this bug a regression?: ---
Last known good build:


Attachments
Sample Repro Project (905.26 KB, application/x-zip-compressed)
2017-06-01 21:39 UTC, Sid Dubey
Details
minimized repro (300.35 KB, application/x-zip-compressed)
2017-06-13 16:59 UTC, Jimmy [MSFT]
Details

Description Sid Dubey 2017-06-01 21:38:24 UTC
In our app we have a problem of viewmodels instantiated twice. 

The reason this is happening is that creating a new Page instantiates a new viewmodel and then pushing the page on the Navigation stack creates the second viewmodel.In the Page XAML we are referencing a view model locator. This viewmodelLocator has a property of the view model and the getter of this property returns a new viewmodel via a resolve in the dependency container (Unity). So, the problem in fact is that pushing the page on the navigation stack using Navigation.PushAsync(page, true) gets a new view model because somehow the bindings reference the viewmodellocator which in turn get the new viewmodel. 

Side information : The dependency container registers viewmodels in a Transient way, not “static”.

Am I wrong to rely on the concept of resolving a new viemwodel each time the MyViewModel property gets called? I don’t want to have a direct reference to the viewmodels because keeping these references means that the Garbage Collector will not release them and the services/repository that gets injection also are not released.

Or does the Navigation.PushAsync() does something that is not correct?
Funny thing is that Navigation.PushModelAsync() works as expected and does not have the side effect of creating a new viewmodel.

Browsing the internet I could only find one other person with the same problem.
https://forums.xamarin.com/discussion/49257/view-model-instantiating-multiple-times

One solution is to remove the BindingContext to the viewmodellocator from the XAML and putting it in the code behind, but I think this will kill my possibility to have live data in the Xamarin Previewer. Am I Right?

Parts of my code for a better understanding of the problem:

Viewmodellocator

public CarsViewModel CarsVIewModel 
{
	get
	{
		return _dependencyContainer.Resolve<CarsViewModel>();
	}

}

CarsPage

BindingContext=“{Binding CarsViewModel, Source={StaticResource ViewModelLocator}}"

NavigationService

var page = Actvator.CreateInstance(type) as BasePage;
await (page.BindingContext as BaseEventsViewModel).Initialize();
await (page.BindingContext as BaseEventsViewModel).LoadDataAsync();
await Navigation.PushAsync(page, true);

Can you tell me why the last line of code calls the ViewModelLocator (again) and gets me a new ViewModelLocator ?
Do I need to strip the BindingContext from the XAML and put it in the code behind? Or do I need to resolve the view model in my NavigationService? And if I strip the BindingContext from the XAML and put in the code behind, would the Xamarin Previewer Still work? 

If it might be the case that this is a known bug that will or can not be fixed in the near future, I would like to know what the next best solution would be and, maybe a more interesting question : Are we wrong for wanting the view models and every injected class to NOT live long? Our current app is a reasonably big App with lots of functionality and lots of view models etc. that would all be in memory.

References : https://bugzilla.xamarin.com/show_bug.cgi?id=45557 
           : https://github.com/xamarin/Xamarin.Forms/pull/470

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Attachment contains a Repro Project. 

Steps to Reproduce. 

1. Open the solution in Xamarin Studio 
2. Run on iOS Simulator
3. Type something in the username and password entries.
4. Place a breakpoint in the constructor of the CarsViewModel and you will see that the Breakpoint is hit more than once
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IDE Version 

=== Xamarin Studio Enterprise ===

Version 6.3 (build 864)
Installation UUID: da052796-640d-4f51-9571-65cc0968c4b9
Runtime:
	Mono 5.0.1.1 (2017-02/5077205) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 500010001

=== NuGet ===

Version: 3.5.0.0

=== Xamarin.Profiler ===

Version: 1.5.4
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Xamarin.Android ===

Version: 7.3.1.2 (Visual Studio Enterprise)
Android SDK: /Users/Siddharth/Library/Android/sdk
	Supported Android versions:
		4.0.3  (API level 15)
		4.1    (API level 16)
		4.2    (API level 17)
		4.3    (API level 18)
		4.4    (API level 19)
		4.4.87 (API level 20)
		5.0    (API level 21)
		5.1    (API level 22)
		6.0    (API level 23)
		7.0    (API level 24)
		7.1    (API level 25)

SDK Tools Version: 26.0.1
SDK Platform Tools Version: 25.0.5
SDK Build Tools Version: 23.0.3

Java SDK: /usr
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Xamarin Inspector ===

Version: 1.2.2
Hash: b71b035
Branch: d15-1
Build date: Fri, 21 Apr 2017 17:57:12 GMT

=== Apple Developer Tools ===

Xcode 8.3.2 (12175)
Build 8E2002

=== Xamarin.iOS ===

Version: 10.10.0.36 (Visual Studio Enterprise)
Hash: d2270eec
Branch: d15-2
Build date: 2017-05-22 16:30:53-0400

=== Xamarin.Mac ===

Version: 3.4.0.36 (Visual Studio Enterprise)

=== Build Information ===

Release ID: 603000864
Git revision: 6c2f6737278ccc3e81e12276d49c0d92f975f189
Build date: 2017-04-24 11:26:01-04
Xamarin addins: d8d46e577d8507c35260ce9d73df3c33415bb214
Build lane: monodevelop-lion-d15-1

=== Operating System ===

Mac OS X 10.12.5
Darwin Siddharths-MBP.attlocal.net 16.6.0 Darwin Kernel Version 16.6.0
    Fri Apr 14 16:21:16 PDT 2017
    root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64
Comment 1 Sid Dubey 2017-06-01 21:39:13 UTC
Created attachment 22635 [details]
Sample Repro Project
Comment 3 Jimmy [MSFT] 2017-06-06 17:53:41 UTC
I can reproduce this issue using the attached repro project.

I set a break point on line 58 in CarsViewModel.cs in order to compare the call stack of both calls. The first call is coming from the initial loading of the XAML[1]. The second time seems to come from Element.OnChildAdded being called as part of the page being pushed[2].

This is affecting iOS and Android, and presumably UWP too since this seems to be coming from code in the Core assembly. I'm confirming this report so the engineers can look investigate further.

[1] https://gist.github.com/jimmgarrido/fd54fdbacc207e06395acf981f4b27bf#file-call-1-L29
[2] https://gist.github.com/jimmgarrido/fd54fdbacc207e06395acf981f4b27bf#file-call-2-L20-L28
Comment 4 Stephane Delcroix 2017-06-09 19:13:45 UTC
https://github.com/xamarin/Xamarin.Forms/pull/983
Comment 5 Stephane Delcroix 2017-06-13 07:48:49 UTC
@Sid I'd like to be sure I understand your questions correctly

> Can you tell me why the last line of code calls the ViewModelLocator (again) and gets me a new ViewModelLocator ?
I don't get this. No new VMLocator should be created, or could be, as the .ctor is private. I guess you mean "and gets a new CarsViewModel" ?

This happens because when the page is added to the parent, the parent BindingContext is set and the Bindings are recomputed, even though they won't change as the Source is set.
I'm working on a fix so that doesn't happens anymore. You will still get a new CarsViewModel every time you instantiate a new CarsPage. If you want your CarsViewModel to be shared across page instances, you're responsible for that in your VM Locator.

Btw, I can't run your project on iOS. I get 

Resolution of the dependency failed, type = "DBTestApp.ViewModels.Cars.CarsViewModel", name = "(none)".
Exception occurred while: while resolving.
Exception is: InvalidOperationException - The current type, DBTestApp.Service.Services.ICarsService, is an interface and cannot be constructed. Are you missing a type mapping?
-----------------------------------------------
At the time of the exception, the container was:

  Resolving DBTestApp.ViewModels.Cars.CarsViewModel,(none)
  Resolving parameter "carsService" of constructor DBTestApp.ViewModels.Cars.CarsViewModel(DBTestApp.Service.Services.ICarsService carsService)
    Resolving DBTestApp.Service.Services.ICarsService,(none)
Comment 6 Johan Lezwijn 2017-06-13 08:03:11 UTC
@Stephane Delcroix 

Since Sid is asking in behalf of me :

The question is indeed about the viewmodel being instantiated multiple times, not the VMLocator itself as the VMLocator is a singleton with a private constructor. 

I am fully aware that if I would want to re-use a viewmodel for multiple views/pages,  that I am responsible for that in the VMLocator itself.

I am sorry to hear that the sample project does not work. Can I help to het it fixed? It worked for me on iOS when I uploaded it.
Comment 7 Jimmy [MSFT] 2017-06-13 16:59:56 UTC
Created attachment 22863 [details]
minimized repro

Since the issue is not directly caused by using a dependency container, I've minimized the project to remove this and still reproduce the issue.


### Steps to Reproduce
1. Run the attached project
2. Press "Go to Test Page"


### Actual Results
The debug output will indicate that the TestViewModel ctor was called twice when navigating to the page.
Comment 8 Stephane Delcroix 2017-06-14 09:13:01 UTC
Thanks @Jimmy. I was able to run your sample and reproduce the issue
Comment 9 Stephane Delcroix 2017-06-14 11:31:43 UTC
*** Bug 45557 has been marked as a duplicate of this bug. ***
Comment 10 lecnier.freeman 2017-08-18 14:20:30 UTC
Can anybody comment on the status of this bug?
Comment 11 Evgeny Zborovsky 2017-09-05 07:45:07 UTC
Here is my thread on stackoverflow about the same bug: https://stackoverflow.com/questions/42771108/xamarin-forms-viewmodellocator-get-called-twice

I had to drop the VieModelLocator and to use the next workaround:
<ContentPage.BindingContext>
    <local:TestViewModel />
</ContentPage.BindingContext>

The bug is still exists in latest and greatest XF 2.3.4.270

Note You need to log in before you can comment on or make changes to this bug.