johnpatrickmorgan / NavigationBackport

Backported SwiftUI navigation APIs introduced in WWDC22
MIT License
865 stars 53 forks source link

Fatal error: No view builder found for key when changing tabs. #33

Closed damiantrimboli closed 1 year ago

damiantrimboli commented 1 year ago

Hello! I'm having a crash when I push a View (using the nbNavigationDestination with isPresented) and then switching to another tab. It looks like that when the root view is no longer visible, the deinit in the LocalDestinationBuilderModifier is called, so the removeLocalBuilder is called for all the destinations and ends up with an empty builders array, then, for some reason it tries to build a navigation destination again, and crashes because the builders array is empty.

Any idea of how can I workaround this?

johnpatrickmorgan commented 1 year ago

Thanks for raising this @damiantrimboli! In the course of investigating this, I realised there was a regression in version 0.7.3 that might reset the navigation stack when NBNavigationStack re-appears. I've fixed that in version 0.7.4, but it sounds like your issue is slightly different. I'll continue to investigate, but if you were previously using 0.7.3 could you check if you still see the issue in 0.7.4 please? Thanks!

johnpatrickmorgan commented 1 year ago

I might need a little bit more context or example code please @damiantrimboli. The example app in this repo uses a TabView and I'm not able to replicate the issue - switching to another tab does not trigger the deinit in LocalDestinationIDHolder:

https://github.com/johnpatrickmorgan/NavigationBackport/blob/88437a4bd67dc2c0222d8def073d0eb3a38a844a/NavigationBackportApp/Shared/ContentView.swift#L41-L48

damiantrimboli commented 1 year ago

I just tested with the latest version and the issue is still happening. My team built a custom TabView, and when you change the tab, it switches the currentView displayed.

damiantrimboli commented 1 year ago

So, in the viewModel:

 @Published var selectedTab: Tab {
    didSet {
      switch selectedTab {
      case .home:
        currentView = homeView
      case .map:
        currentView = mapView
      case .settings:
        currentView = settingsView
      }
    }

  private let homeView: AnyView
  private let mapView: AnyView
  private let settingsView: AnyView
}

All the views are instantiated in the init, so they are all stored and persisted in memory. in the View, we just show viewMode.currentView

damiantrimboli commented 1 year ago

Think of it as it is a change in the context. Like from registration flow to home view.

damiantrimboli commented 1 year ago

hello @johnpatrickmorgan! any update on this? I need to release an update soon and I would need to stop using this library if I don't find any workaround at least, I would prefer to keep it!

johnpatrickmorgan commented 1 year ago

Hi @damiantrimboli - thanks for clarifying...

Think of it as it is a change in the context. Like from registration flow to home view.

I see, so I guess you're OK with the state resetting when switching from one tab to another? Unlike with TabView, which would preserve state when you switch between tabs?

As far as the crash goes, I've been unable to reproduce it so far. I tried to reproduce it in the example app: https://github.com/johnpatrickmorgan/NavigationBackport/commit/cceddc52f5b427287cfc84207d23833455e2d605. If I switch to the NavStack tab, pop to root, and tap 'push local destination' (which uses the $isPresented API), then switch to the Home tab, there's no such crash.

removeLocalBuilder is called as expected but it doesn't try to build a navigation destination as you described:

then, for some reason it tries to build a navigation destination again

I guess there's something I'm missing that's contributing to the issue. Are you able to provide a minimal reproduction please?

damiantrimboli commented 1 year ago

hey! @johnpatrickmorgan I'm attaching a sample project that I could reproduce the issue. Note that the crash only happens in iOS 15.5, not on iOS 16 (I don't use this library for iOS 16) Repro steps are: In Tab 1, press the Push View button so a view is pushed, and then switch to Tab 2 and that's it!

In this case is happening in this custom Tab implementation, but I'm sure there plenty of other use cases that can reach this crash.

ReproCase.zip

damiantrimboli commented 1 year ago

@johnpatrickmorgan was it useful? do you need anything else from me? I'm happy to help!

johnpatrickmorgan commented 1 year ago

Hi, thanks for the sample project - I hadn't tested on iOS 15 so that's probably why I couldn't reproduce it. I'll investigate this as soon as I can, hopefully tonight.

damiantrimboli commented 1 year ago

thank you! the crash also happen on simulator too.. that makes it easier to reproduce.

I thought this library was intended to be used in iOS versions below 16, because there's even a deprecated warning when used in iOS 16.

johnpatrickmorgan commented 1 year ago

Hi @damiantrimboli, I investigated the issue and was able to reproduce on iOS 15. It seems there are some extraneous re-renders on iOS 15 after the navigation stack is removed from the view tree which fail because the local builder function has already been cleaned up at that point. Dispatching the cleanup asynchronously was enough in my testing to overcome the issue. The change is available in version 0.7.5. Please let me know if that doesn't resolve the issue for you.

johnpatrickmorgan commented 1 year ago

I thought this library was intended to be used in iOS versions below 16, because there's even a deprecated warning when used in iOS 16.

@damiantrimboli The library can be used on iOS 14, 15 and 16. The deprecation warning will only be triggered if the project's minimum deployment target is iOS 16. In those cases, the built-in SwiftUI NavigationStack can be used, hence the warning.

damiantrimboli commented 1 year ago

thank you! will check!

damiantrimboli commented 1 year ago

@johnpatrickmorgan it looks like it is working indeed, I will run a couple more tests to confirm! but looks good!

ilendemli commented 1 year ago

@johnpatrickmorgan ~I encountered this issue right now after updating to 0.8.0. We too use a custom TabView. I took @damiantrimboli example project and changed it to represent the way we use it in our App (we are against Views in Models). In our App, we deeplink into the app which changes the tab first (if necessary, and popsToRoot by setting path to an empty array, also if necessary), and then is supposed to push a view which causes the crashes. I attached an example: Archive.zip~

never mind, in the example, the nbNavigationDestination modifier was missing