Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shell, Navigation and Page instantiation #9300

Closed
pfafft33 opened this issue Aug 10, 2022 Discussed in #5810 · 52 comments
Closed

Shell, Navigation and Page instantiation #9300

pfafft33 opened this issue Aug 10, 2022 Discussed in #5810 · 52 comments
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout p/2 Work that is important, but is currently not scheduled for release s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@pfafft33
Copy link

pfafft33 commented Aug 10, 2022

Discussed in #5810

Originally posted by ioiooi April 4, 2022
Hello,

are Pages that are defined in the visual hierarchy only ever instantiated once? And if that is the case is there a way to clear the cache and force the creation of a new Page instance the next time I navigate to it?

For example the following hierachy is described in a AppShell.xaml:

<ShellContent Route="LoginPage" Shell.FlyoutBehavior="Disabled" ContentTemplate="{DataTemplate local:LoginPage}" />

<FlyoutItem FlyoutDisplayOptions="AsMultipleItems">
    <ShellContent Title="About" Icon="tab_about.png" Route="AboutPage" ContentTemplate="{DataTemplate local:AboutPage}" />
    <ShellContent Title="Browse" Icon="tab_feed.png" ContentTemplate="{DataTemplate local:ItemsPage}" />
</FlyoutItem>

<ShellContent Route="SecretPage" Shell.FlyoutBehavior="Disabled" ContentTemplate="{DataTemplate local:SecretPage}" />

The LoginPage constructor gets called when the App is started. Navigate to the AboutPage with Shell.Current.GoToAsync($"//{nameof(AboutPage)}") and its constructor gets called.

Now lets say the AboutPage has a Logout-Button which does some things and at the end it calls Shell.Current.GoToAsync($"//{nameof(LoginPage)}"). The LoginPage constructor does not get called. Navigate back from the LoginPage to the AboutPage and its constructor also does not get called.

The same deal if I would navigate from the LoginPage to the SecretPage. The constructor of SecretPage would only get called the first time.

Lets say I use ViewModels for all those Pages. I can't really create a state in the ViewModel constructor because the Page only gets instantiated once and so in my case the ViewModel also only gets instantiated once. I would need to do the work in a OnAppearing.

I would want for the Pages after the LoginPage to always be newly constructed in order to make sure that theres a clean state.

PS:
A lot of examples also contain DetailsPages... something like ItemsDetails, MonkeyDetails, CatDetails etc... Those Pages are never declared in the AppShell.xaml but are registered like so

Routing.RegisterRoute("monkeys/details", typeof(MonkeyDetailPage));

and from what Ive seen seem to be always instantiated every single time theyre navigated to.

Workaround

#9300 (comment)

@pfafft33
Copy link
Author

I have run into this issue as well, and it's a BIG one!
Also, I just upgraded to VS Community 2022 Preview v 17.4.0 Preview 1.0 hoping that it would be fixed.
Unfortunately, it was NOT fixed in this latest update.

I have had to resort to creating a "REFRESH PAGE" button at the top of each non-detail page, that executes the same code as the Constructor. This is the ONLY way I have found to get my data updated on the non-detail pages.

PLEASE, PLEASE, PLEASE fix this ASAP. There is NO WAY that I'm going to release a Production app that requires my users to click a REFRESH PAGE button every time that they navigate to a non-detail page.

@angelru
Copy link

angelru commented Aug 10, 2022

I faced the same "issue", I ended up overriding the OnNavigated and OnNavigating in AppShell. There I get the navigation path and then execute the method of my ViewModel.

@pfafft33
Copy link
Author

@angelru Thanks for the tip!

@PureWeen
Copy link
Member

@pfafft33
Copy link
Author

@PureWeen - I tried OnNavigatedTo like you suggested. I put the following code into my StoreListPage.xaml.cs file:

protected virtual void OnNavigatedTo(NavigationEventArgs e)
{
    _viewModel.StoreListViewModelConstructorCode();
}

And, I set a breakpoint and ran my App.
As it turns out, this method does NOT get called when I navigate to my Page.

@pfafft33
Copy link
Author

@angelru - I tried your suggestion. I went into my AppShell.xaml.cs file and typed in the following code:

public override void OnNavigated()
{

}

public override void OnNavigating()
{

}

I got Compiler Error CS0115: no suitable method found to override

By your message, it sounds like you got this to work. What am I missing?

@angelru
Copy link

angelru commented Aug 11, 2022

This code is from an application in Xamarin Forms, in .NET MAUI I have not tried.

        protected override void OnNavigated(ShellNavigatedEventArgs args)
        {
            string location = args.Current?.Location?.ToString();
            string previous = args.Previous?.Location?.ToString();

            if (location is "//Main/Workouts")
            {
                switch (previous)
                {
                    case "//MainPage":
                    case "//MainPage/Login":
                    case "//Main/Workouts/WorkoutQuestionnaire":
                        _ = WorkoutsPageViewModel.Current.RunSafeAsync(WorkoutsPageViewModel.Current.InitWorkoutsAsync);
                        break;
                }
            }

            base.OnNavigated(args);
        }

        protected override void OnNavigating(ShellNavigatingEventArgs args)
        {
            string location = args.Target.Location.ToString();

            if (location == "//Main")
            {
                vm.InitHankuk();
                LocalNotification();
            }

            base.OnNavigating(args);
        }

@pfafft33
Copy link
Author

@angelru - Using your example code, I was able to get it to work with just the following code added to AppShell.xaml.cs

protected override void OnNavigated(ShellNavigatedEventArgs args)
{
    string location = args.Current?.Location?.ToString();

    if (location is $"//{nameof(StoreListPage)}")
    {
        ((StoreListPage)Shell.Current.CurrentPage)._viewModel.StoreListViewModelConstructorCode();
    }

    base.OnNavigated(args);
}

Thanks SO much for your help!

I STILL think that this is a bug, and that it should get fixed ASAP.
We shouldn't have to resort to this kind of work-around. A page should automatically get refreshed / re-instantiated each time it is navigated to.

@PureWeen
Copy link
Member

Related #7354

@PureWeen PureWeen added this to the Backlog milestone Aug 11, 2022
@PureWeen PureWeen added the area-controls-shell Shell Navigation, Routes, Tabs, Flyout label Aug 11, 2022
@PureWeen PureWeen modified the milestones: Backlog, .NET 8 Planning Aug 11, 2022
@dotnet dotnet deleted a comment Aug 11, 2022
@PureWeen
Copy link
Member

@pfafft33 can you log a bug?

From my tests it gets called.

image

@MatthewB05
Copy link

OnNavigatedTo is broken as soon as you have tabs within the flyout item, as per the example at the top of this issue. It works for the first tab, but not on any other tab, until you go to a different flyout item, and then back again, at which point the OnNavigatedTo will fire for whichever tab was previously shown. Certainly the case with both Windows and Android on latest preview downloaded on the date of this comment.

@AlexanderSCK
Copy link

You need to register your pages and the viewmodels associated with the pages as either Singletons or Transient in MauiProgram.cs. If regestered as Singleton the page is only instantianted once and the state of the page and the viewmodel is kept, where as for details pages for example you would use transient so its creted and destroyed each time you navigate to/back from it. Also use the EventToCommand behavior from the CommunityToolkit to call the pages "Appearing" and bind your code to a command in the viewmodel. Register the viewmodels the same as the pages in the MauiProgram.cs file.

@papafe
Copy link

papafe commented Dec 19, 2022

@AlexanderSCK It seems that when using Shell, the pages are reused even if using Transient, so unfortunately this does not solve the issue.

@EntityBox
Copy link

It seems like only the AppShell pages that has the routing attribute in xaml has this issue, if you do a second page navigation, the DI works as expected with the Singleton/Transient. Creating methods to perform the functions of the constructor is a workaround but not the best solution.

@shadowfoxish
Copy link

Perhaps what is needed is a way OnDisappearing or a navigation event handler detecting "Pop" or "Go Back" to set a flag to indicate whether the view should be destructed or if it can be reused?

On the positive side, navigation is much much quicker than with Stack based navigation from XF. Using good practices and not storing a lot of state in the views themselves helps. Using OnAppearing in conjunction with a call to your view model and ApplyQueryAttributes (from IQueryAttributable on the ViewModel classes) in should let reused instances reset themselves.

@fgiacomelli
Copy link

I have a similar requirement in a new app, login page with registration flow, and then a list of pages under a tab menu.

I have registered some pages and viewmodels under the tabs as Transient, and I expected that navigating to that pages they should reconstructed as new instances. This is because I can logout with a user, returning to the login page (setting it as root and clearing in this way the navigation stack), and relogging with another user, and I need that pages and viewmodels be reinitialized.

But what I see at this time is that the pages and viewmodels are never recreated. I did not registered the route to this pages using the appshell.xaml, but forcing the route registration in appshell.xaml.cs (code behind).

I found a workaround declaring a property in a baseViewmodel of each viewModel that I use to understand if call the Initialize method of each and resetting it from codebehind, forcing a sort of reinitialization when the page appears, but it's very tricky, and for use it I need to declare all the pages and viewmodels as singleton.

@fgiacomelli
Copy link

It seems like only the AppShell pages that has the routing attribute in xaml has this issue, if you do a second page navigation, the DI works as expected with the Singleton/Transient. Creating methods to perform the functions of the constructor is a workaround but not the best solution.

in my situation I did not register the route from xaml but from code behind, but the pages are never recreated

@EntityBox
Copy link

EntityBox commented Feb 1, 2023

in my situation I did not register the route from xaml but from code behind, but the pages are never recreated

Then it might be a routing issue in general. seems like GoToAsync($"//{nameof(SomePage)}") the // is suppose to remove everything from the NavigationStack and in theory dispose of any Transient pages, but this does not happen.

Having a look at MAUI Docs the wording use is "replace the navigation stack", maybe this should be the hint that it does not dispose on "replace"?

@PureWeen - This is an issue if DI Transient Pages and ViewModels are not disposed. Do we perhaps have estimated fix, PR or Merge on this problem.

@fgiacomelli
Copy link

in my situation I did not register the route from xaml but from code behind, but the pages are never recreated

Then it might be a routing issue in general. seems like GoToAsync($"//{nameof(SomePage)}") the // is suppose to remove everything from the NavigationStack and in theory dispose of any Transient pages, but this does not happen.

It could be, but if you look in the Navigation stack after Navigated to the // page you can see that it just contains the root page.

In the meantime I just created a sample to reproduce the problem, so if we are misunderstanding some concepts of DI or AppShell navigation somebody could help us, because I really can't understand if I'm doing something wrong, if I have a wrong expectation of if this is another bug of MAUI.

In this app (that it's really raw because I just modified another repo created to reproduce a different bug of MAUI) the MainPage represent the LoginPage, pressing a button we navigate to a tabbed page, where the "OtherPage" Page is registered as transient as its viewmodel. I can see that the viewmodel and page constructors are called only once, then returning to the mainPage and after to the "OtherPage" I would expect to see new instances but it doesn't happen.

Here is the repository: https://github.com/fgiacomelli/DependencyInjectionOnTransient

@angelru
Copy link

angelru commented Feb 2, 2023

Doesn't it make sense for views to be reused? In my opinion it would be faster for navigation. But ViewModels shouldn't be like this, since in them we get data and have changes to the views. But I understand that we can't have the view as a singleton and a ViewModel associated with that view as a transient, because this would also be a singleton.

What would be the correct approach then?

@fgiacomelli
Copy link

Sure that it makes sense, but like in any container we must be able to configure the classes registered as singleton or new on each request, and the architecture need to implement the correct behavior

@angelru
Copy link

angelru commented Feb 3, 2023

Sure enough, I have a page added in AppShell and I registered it as a Transient next to its ViewModel, and the constructor is only called 1 time when I navigate to it.

@PureWeen
Copy link
Member

PureWeen commented Feb 3, 2023

in my situation I did not register the route from xaml but from code behind, but the pages are never recreated

Then it might be a routing issue in general. seems like GoToAsync($"//{nameof(SomePage)}") the // is suppose to remove everything from the NavigationStack and in theory dispose of any Transient pages, but this does not happen.

Having a look at MAUI Docs the wording use is "replace the navigation stack", maybe this should be the hint that it does not dispose on "replace"?

@PureWeen - This is an issue if DI Transient Pages and ViewModels are not disposed. Do we perhaps have estimated fix, PR or Merge on this problem.

Yea :-/ this was definitely a very unfortunate oversight. I think some users will want GoToAsync($"//{nameof(SomePage)}" to clear the navigation stack and others won't. According to the docs and the rules of the system it should clear the stack and the fact that it doesn't is confusing. I'm still thinking through various paths here to enable better intention here.

Perhaps just tying into Transient/scoped/singleton here is enough. If you register as Transient, it'll start you over if you don't it'll maintain the stack.

No specific ETA on this, but I'm going to see if I can work out some helpers to enable the features here. Get some active feedback going!

@lemcoder
Copy link

lemcoder commented Feb 4, 2023

I've created a workaround using ServiceProvider class which is avaliable via DI in MAUI project. I've defined a generic TransientContentPage which resolves viewModel when OnAppearing event occurs:

public class TransientContentPage<TViewModel> : ContentPage
  {
      private readonly IServiceProvider serviceProvider;
      public TransientContentPage(IServiceProvider serviceProvider)
      {
          this.serviceProvider = serviceProvider;
      }

      protected override void OnAppearing()
      {
          BindingContext = serviceProvider.GetService<TViewModel>();
          base.OnAppearing();
      }
  }

This approach seems to work fine for me, still it is far from perfect.

@samhouts samhouts modified the milestones: .NET 8 Planning, Under Consideration, .NET 8 SR3 Sep 27, 2023
@homeyf
Copy link

homeyf commented Nov 7, 2023

Verified this issue with Visual Studio Enterprise 17.8.0 Preview 5.0. Can repro on windows platform.
https://github.com/PureWeen/ShanedlerSamples
image

@homeyf homeyf added s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage labels Nov 7, 2023
@Redth Redth modified the milestones: .NET 8 SR3, .NET 8 + Servicing Jan 10, 2024
@robertok77
Copy link

I would appreciate if ContentCache would be an option ...

@fekberg
Copy link

fekberg commented May 23, 2024

Any updates on when this is planned to be fixed @samhouts?

@PureWeen PureWeen added p/2 Work that is important, but is currently not scheduled for release and removed p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint labels Jul 2, 2024
@robertogonzalezdelarosa
Copy link

in my situation I did not register the route from xaml but from code behind, but the pages are never recreated

Then it might be a routing issue in general. seems like GoToAsync($"//{nameof(SomePage)}") the // is suppose to remove everything from the NavigationStack and in theory dispose of any Transient pages, but this does not happen.

Having a look at MAUI Docs the wording use is "replace the navigation stack", maybe this should be the hint that it does not dispose on "replace"?

@PureWeen - This is an issue if DI Transient Pages and ViewModels are not disposed. Do we perhaps have estimated fix, PR or Merge on this problem.

this is true it happens only if we navigate using // to clear navigation stack. Trasient pages are clear out if we navigate without //

@Lucasian20
Copy link

Lucasian20 commented Aug 2, 2024

Is there any updates on when this is getting fixed. This issue is two years old why is this not been made a priority. This is a showstopper for me and I'm guessing it is for many others. I tried @PureWeen solution like @pikausp did and same it creates strange behaviour. It initially did work until I tried to navigate back again to that same shell page the app crashes with unknown exception. This is a very hacky approach which i would like to avoid. If anyone has managed to figure out a solution that does work then please post.

@robertogonzalezdelarosa
Copy link

robertogonzalezdelarosa commented Aug 2, 2024 via email

@Lucasian20
Copy link

Lucasian20 commented Aug 2, 2024

@robertogonzalezdelarosa Yeah this is the solution I have already tried and is not working for me on version 8.0.70 - It would be nice to get a two year old problem fixed, not to hack it in.

@robertogonzalezdelarosa
Copy link

robertogonzalezdelarosa commented Aug 2, 2024 via email

@ederbond
Copy link
Contributor

ederbond commented Aug 2, 2024

I have reported similar issue around the disposal of View and ViewModels +2 years ago like you can see here:
#7354
But instead of making it a Top priority and fix it right way MSFT just closed my issue ticket (without fixing anything) and just created other 2 tickets to think about a spec to resolve this problem.
I'm glad that my projects have switched focus to WPF for now cause I was tired about MAUI managers incompetence to prioritize such showstopper bugs like this.

@ederbond
Copy link
Contributor

ederbond commented Aug 2, 2024

What is the state of these related issues:
#21814
#21816
@PureWeen @davidortinau ?

@ederbond
Copy link
Contributor

ederbond commented Aug 2, 2024

It has been +2 years since I've reported this bug for MSFT and they didn't come up with a proper solution, so to save your precious time consider completely ditching MAUI AppShell for now, and use the following library to workaround this issue:
https://github.com/naveasy/Naveasy

@PureWeen PureWeen modified the milestones: .NET 8 + Servicing, Backlog Aug 2, 2024
@Lucasian20
Copy link

It has been +2 years since I've reported this bug for MSFT and they didn't come up with a proper solution, so to save your precious time consider completely ditching MAUI AppShell for now, and use the following library to workaround this issue: https://github.com/naveasy/Naveasy

This is a great solution, but is just what I was afraid of. Our previous App in Xamarin we were using Prism but decide for Maui we would drop that and start using Shell. We also have done the same and copied from Prism the functionality we need but using both the Shell and Stack Navigation in a NavigationService.cs, which I see you've gone that step further using Navigation page and removing AppShell.

It's a shame cause it was going quite well. I found out early on the issue with the memory leak on the stack - pages not being removed/cleared #12354 so I decided to adopt both. To home screen via a login/landing view I navigate Shell.Current.GoToAsync and we have various paths of navigation via the stack Push and Pop, which as the shell navigation replaces the stack it was the cleanest way to clear the stack without getting unknown exceptions, when going home.

We have a new feature in this App that allows us to update the style of the App, by logging out and back In again. This works great but as the shell navigated pages/views only instantiate once in the lifetime the new updated style of course will not change. The constructors of the view and it's viewmodel need to be hit every time.

@Lucasian20
Copy link

Any updates on when this is planned to be fixed @samhouts?

@PureWeen
Copy link
Member

PureWeen commented Sep 6, 2024

This is done for NET9
#23863

if you register your pages as transient it will recreate them

@PureWeen PureWeen closed this as completed Sep 6, 2024
@Lucasian20
Copy link

This is done for NET9 #23863

if you register your pages as transient it will recreate them

Any chance it could be put into .NET8, or are there no more planned releases before .NET9. We were not going to bother with 9 as it's standard term support and wait for 10.

@Vroomer
Copy link

Vroomer commented Sep 9, 2024

@Lucasian20 There is no such thing as standard term support in MAUI. You should always update to new version to keep up to date with iOS/Android API.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout p/2 Work that is important, but is currently not scheduled for release s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests