obvios / Routing

Routing library for abstracting navigation logic from SwiftUI views.
MIT License
67 stars 6 forks source link

Memory issue on Navigation stack, Sheet & FullSheet #1

Closed Suri1044 closed 7 months ago

Suri1044 commented 7 months ago

Hi Team, I have observed that the package manager has memory issues. I have created the sample project and included the routing package manager to test the memory and its increasing the memory on all 3 navigation cases.

I am attaching the video i have created and the sample project for reference purpose. please check and fix the issue and let me know the cause.

https://github.com/obvios/Routing/assets/68360396/8c5fb7cb-22e1-405f-ac03-c574fddf0726

RoutingProject.zip

Need Help on: Also need help on how to create multiple Routables ie, enum LoginRoute: Routable, enum AccountRoute: Routable, enum TabRoute: Routable and include this routables as view hieararchy using
ie, RoutingView(LoginRoute.self) { route in MainView(router: route) } RoutingView(AccountRoute.self) { route in MainView(router: route) } RoutingView(TabRoute.self) { route in MainView(router: route) }

Let me know how to connect with you to discuss the issues. Thanks in advance.

obvios commented 7 months ago

Hi @Suri1044 ,

Regarding your question using multiple RoutingView instances, I would recommend separating them using TabView. For example:

TabView {
    RoutingView(LoginRoute.self) { route in
         MainView(router: route)
    }
    RoutingView(AccountRoute.self) { route in
         MainView(router: route)
    }
}

Each RoutingView can only support one Routable. So that is why I recommend separating the flows with a tab view.

To create a Routable instance, please follow our example on our README

Suri1044 commented 7 months ago

Hi Team, I have a problem when i have added different RoutingView in TabView. Use Case:

  1. I have loginview where i injected Loginview in app Content view and after login i have screens to navigate (accounts and Tabview)

RoutingView(LoginRoute.self) { route in MainView(router: route) }

  1. When i land on Tabview, I have added different routes as suggested by you above.

ie, TabView {

RoutingView(HomeRoute.self) { route in
     HomeView(router: route)
}
RoutingView(MoreRoute.self) { route in
     MoreView(router: route)
}

}

  1. I am unable to navigate (Home tab root) when i added button from HomeView and trying to push, Nothing working.

  2. Can you please let me know why its not working? Do i need to add the RoutingView in mainApp ContentView only ? i can't add in Tabview as viewhierachy?

Waiting for your reply, I am actively looking on this, if not working i need to go to default way navigationstack. Appreciate your help.

Thanks,

Suresh

On Mon, Feb 5, 2024 at 1:54 AM Eric Palma @.***> wrote:

Hi @Suri1044 https://github.com/Suri1044 ,

Regarding your question using multiple RoutingView instances, I would recommend separating them using TabView. For example:

TabView { RoutingView(LoginRoute.self) { route in MainView(router: route) } RoutingView(AccountRoute.self) { route in MainView(router: route) } }

Each RoutingView can only support one Routable. So that is why I recommend separating the flows with a tab view.

To create a Routable instance, please follow our example on our README

— Reply to this email directly, view it on GitHub https://github.com/obvios/Routing/issues/1#issuecomment-1925903965, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQJRRTEUIZZRODVCGQXG6LLYR7U7XAVCNFSM6AAAAABCV3ZKPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVHEYDGOJWGU . You are receiving this because you were mentioned.Message ID: @.***>

obvios commented 7 months ago

Hello @Suri1044 ,

In order to best help you, could you share more of the relevant code? It is hard for me to provide you with a definite answer from the code you have shared above. I would like to see your ContentView as well as the rest of your views. Could you share the project again?

Thanks,

Eric

Suri1044 commented 7 months ago

CustomTabView 2.zip I have attached the code Eric as requested. Please check and let me know. Injecting routing instance later after login. behaving differently..

Suri1044 commented 7 months ago

I have attached the sample code in git.. Please look into this so that i can use the package manager. Appreciate your help.

On Wed, Feb 7, 2024 at 6:22 PM Eric Palma @.***> wrote:

Hello @Suri1044 https://github.com/Suri1044 ,

In order to best help you, could you share more of the relevant code? It is hard for me to provide you with a definite answer from the code you have shared above.

Thanks,

Eric

— Reply to this email directly, view it on GitHub https://github.com/obvios/Routing/issues/1#issuecomment-1931986853, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQJRRTFSSJFYK73IBMICWCLYSN2J3AVCNFSM6AAAAABCV3ZKPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZRHE4DMOBVGM . You are receiving this because you were mentioned.Message ID: @.***>

obvios commented 7 months ago

Thank you for sharing the project, I will look into it and get back to you with my findings.

Suri1044 commented 7 months ago

@obvios : Any luck with the finding. Do you have any sample which use multiple routes in app for login, account, tab and works. That will be really helpful.

obvios commented 7 months ago

Hi @Suri1044 ,

I looked over your project and concluded that the issues you are facing are not being caused by the Routing library. The issue is how you are structuring your views. You cannot nest a RoutingView inside another routing view.

This is why i suggested using a TabView to separate the RoutingViews.

You can do the following:

// This is okay
TabView {
     RoutingView {
          ...
    }

     RoutingView {
          ...
    }
}

You cannot do this:

RoutingView(ExampleRoute) { router in
      // You cannot use MainView here because it has RoutingView inside it.
      MainView()
}

MainView {
     body: some View {
         // Routing View
          RoutingView { . . .}
     }
}
Suri1044 commented 7 months ago

Hi Obvios/Routing @obvios , I did add in the customTabview in the code which i provided, not inside another routing view (not nesting). Attached screenshot for reference, Also can you make it workable of my working code?/share any multi route code sample plz.

Can you please confirm that can it will wrok with multiple routes or not if i use your package Manager. If yes can you share any code reference plz, i want to understanding how its working in multiple routes, because i don't think its working (i may be wrong)....

Question 2

  1. will same view ex: Account view can work with multiple routes (pushing from multiple routes) like Route1: Login -> AccountView and Route2: TabView -> AccountView

  2. Can you please confirm if i have loginView which is inside Routing View and used for authenication flow (push) and after success can i add routingviews for customTabViews or not? Hard to understand with multiple routes.. and how to integrate in real app.

  3. is there a way we connect plz i am available on number: +91 9884244100.

Regards, Suresh

On Thu, Feb 8, 2024 at 6:56 AM Eric Palma @.***> wrote:

Hi @Suri1044 https://github.com/Suri1044 ,

I looked over your project and concluded that the issues you are facing are not being caused by the Routing library. The issue is how you are structuring your views. You cannot nest a RoutingView inside another routing view.

This is why i suggested using a TabView to separate the RoutingViews.

You can do the following:

// This is okayTabView { RoutingView { ... }

 RoutingView {
      ...
}

}

You cannot do this:

RoutingView(ExampleRoute) { router in // You cannot use MainView here because it has RoutingView inside it. MainView() } MainView { body: some View { // Routing View RoutingView { . . .} } }

— Reply to this email directly, view it on GitHub https://github.com/obvios/Routing/issues/1#issuecomment-1933220628, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQJRRTFEC4QM4UH5EXCZEM3YSQSTRAVCNFSM6AAAAABCV3ZKPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZTGIZDANRSHA . You are receiving this because you were mentioned.Message ID: @.***>

Screenshot 2024-02-08 at 7 14 36 AM
Suri1044 commented 7 months ago

Hi @obvios : Attaching the video with tabview as u suggested as well, Still same issue and also attaching the sample code as well.. It will be good if some sample with multiple route explained.

Anyways can you please look into the issue. Waiting for your reply.

https://github.com/obvios/Routing/assets/68360396/09c8be8c-7f39-48a8-bb6e-6cf8349905a0

TabViewLatest.zip

Suri1044 commented 7 months ago

@obvios : What's the point we can only have one Navigationcontroller for whole application. Its not going to work right? Switching between different routes also plays major role in big application right and we are passing router instances in every view and that's ok with one Navigation since we can push and pop but what if i want to go to another navigation controllers view how to achieve that since we don't have router instance of that navigation here... Overall this will be problem in understanding the package usage without multi route switching...So i request, place a sample/example so that others will follow easily... I almost spent 3 days time (My bad i am not understanding switching routes) with package in implementing and got failured now i dont have other option other than going with the default stack in the application.

obvios commented 7 months ago

Hi @Suri1044 ,

I will include notices on the issues you encountered. I will do my best to provide an example for your use case.

This may take a couple of days, appreciate your patience with this.

obvios commented 7 months ago

@Suri1044 ,

I have modified your project to demonstrate the appropriate way to use the Routing package. Again, the issue is that you were nesting a RoutingView inside another RoutingView. You cannot do this, SwiftUIs NavigationStack won't allow this.

Here is the updated project, please see the comments I added: CustomTabView.zip

https://github.com/obvios/Routing/assets/38594636/9aaa49b2-019a-4401-81d4-d4ff20af4428

I will now close this issue as it is not an issue related to the Routing package, rather it was related to how it was being used. I hope I've helped you understand the issues related to your implementation.

I will also investigate how to support nesting RoutingViews as this would be a great capability.

Suri1044 commented 7 months ago

@obvios Hi Eric, Thanks for the changes and sharing (I figured out this in the morning :) with the rootview issue), and the nested issue you said. But the original issue is about Memory which is getting is still there and that is a big challange. Can you please take a look. Attaching again another video of memory issue which is getting increment when trying on using same tabs over and over again. And earlier also, the first video (code as well attached) is about memory only.

Another use case which need your help in understanding:

  1. Switching between different routes also plays major role in big application right and we are passing router instances in every view and that's ok with one Navigation, since we can push and pop but what if i want to go to another navigation controllers view how to achieve that since we don't have router instance of that navigation here.. ie, what if same view is used by two navigations, how to get the router instances ? do we need create duplicate views/screens one for each ? can't we use same view for both navigations by passing two different route instances in view?
  1. Is there a way we can avoid injecting router instances in every screen ? I am seeing a big pain on this, i know because of different navigation types we have used stateobject instead of EnviromentObject, but still is there a way to create 3 environment variables one for stack, one for sheet and one for fullsheet.

Appreciate your help in taking time and helping me out and in answering queires.

https://github.com/obvios/Routing/assets/68360396/2ca2661f-fae4-41a0-8954-7ff129ef91c0

obvios commented 7 months ago

Hello @Suri1044 ,

I am seeing mixed results on the memory leak you described.

I am seeing my the memory go up initially to about 30mb, however that is all. As I continue to use the app it doesn't continue increasing memory usage.

Can you confirm? Please try several times and let me know if the memory usage is consistently increasing.