NavigationStack + CA issue

[gkaimakas]

Description

I have built a small app with 5 features (A, B, C, X, Y). Features A, B & C are accessible through a NavigationStackABC and when in C a user can present another NavigationStackXY with X that can navigate to Y.

While both navigation stacks work as expected in isolation when I try to present NavigationStackXY from C, the app freezes until getting killed by the OS.

I have created a demo app and found that if instead of creating a view with a Store here, I create a vanilla SwiftUI view everything works as expected.

Checklist

• I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

The app should not freeze.

Actual behavior

The app freezes

Steps to reproduce

1. Download the demo project
2. Run the app (issue is present both on a device and in the simulator)
3. Navigate to any "featC" view using the navigation links..
4. Press "present featureX"

The Composable Architecture version information

0.49.2

Destination operating system

iOS 16.2

Xcode version information

Version 14.2 (14C18)

Swift Compiler version information

swift-driver version: 1.62.15 Apple Swift version 5.7.2 (swiftlang-5.7.2.135.5 clang-1400.0.29.51)
Target: arm64-apple-macosx13.0

[iampatbrown]

Hey @gkaimakas! Thanks for providing a demo project :)

This looks like it is more of a SwiftUI bug and is reproducible with the following:

struct ContentView: View {
  @State var isPresented = false

  var body: some View {
    NavigationStack {
      NavigationLink("Go to destination", value: 42)
        .navigationDestination(for: Int.self) { [foo = {}] _ in // capturing a reference
          Button("Present sheet") {
            isPresented = true
          }
          .sheet(isPresented: $isPresented) {
            NavigationStack {
              NavigationLink("Go to destination", value: "Blob")
                .navigationDestination(for: String.self) { _ in // foo is implicitly captured
                  let _ = foo() // <- Comment out this line
                  Text("Hello, world!")
                }
            }
          }
        }
    }
  }
}

In your example, it would be related to .navigationDestination capturing the store for scoping but it will also occur if the nested .navigationDestination captured any reference. eg:

.navigationDestination(for: FeatureXDestination.self) { [foo = {}] view in
  let _ = foo()
  switch view {
  case .featureY:
    Text("help would be welcome")
  }
}

I'm not sure if this is a known issue or if there is a workaround. Maybe @mbrandonw, @stephencelis or @tgrapperon can help.

I'd encourage you to check out swiftui-navigation if you haven't already. The docs provide some great info on state driven navigation. Incorporating SwiftUINavigation into your demo project would likely fix this issue.

[tgrapperon]

I try to avoid making view properties "jump" across presentation boundaries by capture, because it usually breaks things from my experience. Issues can be even amplified with NavigationStack, as this last one preemptively scouts the view tree ahead to discover .navigationDestinations, independently of the usual view's invalidation process. If you add that "belly" views are unloaded/recreated when navigating more than two screens ahead, you very quickly end in situation that are very hard to understand (not your fault, the way it works silently, until it breaks).
It seems that if you extract the destinations in a standalone View with store and view properties, it works as expected. It's maybe helping by providing a reference point of invalidation to the framework¹, and if it'd work reliably, this could be an easy workaround to your problem.
As Pat suggested, I'd also check swiftui-navigation, as it may already fix the problem at a lower level.

Footnotes

1. When you're passing closures to views, SwiftUI often needs to run the view's body that follows to check what views are effectively built by the closure and the closures it contains. This is systematic for escaping closures, like the ones from presentation/navigation destinations. If you pass stable values that are stored in the view's itself, like the Store instance or the Destination value, SwiftUI can spot that these properties are the same from one run to another, and then infer that the body from this view doesn't need to be re-evaluated, potentially cascading in larger savings. The issue was caused by an infinite invalidation loop BTW, hence the idea of stabilizing things using a dedicated view with stored dependencies. ↩

[gkaimakas]

@iampatbrown @tgrapperon thank you both for your replies.

I've checked SwiftUINavigation but can't exactly wrap my head around on how to achieve both programmatic and user initiated navigation. Guess I will have to experiment a bit more with this.

[tgrapperon]

In any case my workaround wasn't clear, I was suggesting to create:

struct DestinationView: View {
  let store: StoreOf<Root>
  let destination: Destination

  var body: some View {
    switch destination {
      case let .featureB(id):
      … // Same code as in the original implementation
  }
}

and in RootView:

…
  .navigationDestination(for: Destination.self) { destination in
    DestinationView(store: store, destination: destination)
  }

instead of having all your destinations declared in RootView, as it seems it stabilizes things enough to avoid the infinite body loop.

You can totally nest DestinationView into RootView, and if this issues also presents for other users, we can imagine formalizing this in one way or another. I'll definitely use your sample case to experiment with navigation in the future.

AFAIK, SwiftUINavigation doesn't provide helpers for value-based navigation links like in your case, so it can explain why you have difficulties wrapping your head around the problem!
This is rather complex topic, and it partly explains why the navigation update of TCA is taking this long.

As an aside, TCA ships with an utility called SwitchStore that does more or less what you're doing in your .navigationDestination block when you switch/IfLetStore, but I also just noticed how you baked the destinations nesting with scopes, so I'm curious how it'd look in this configuration. I'm not sure it would save a lot here compared to your ad hoc implementation, but it would have maybe fixed the issue coincidentally. I'll check.

[gkaimakas]

@tgrapperon thanks for following up on that! Will check it out!

[dy-kim]

@gkaimakas
1/ You'd be glad to check out ongoing updates on navigation branch
2/ May be you could consider relocating this post to the discussion section. I guess, it seems more like a discussion rather than an issue of TCA :)

[gkaimakas]

@dy-kim would love to move it a discussion but I don't have permission to do so

[mbrandonw]

I'll convert it to a discussion :)