pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.35k stars 1.44k forks source link

Combining Stack-based and Tree-based .navigationDestination navigation leads to infinite loop #2288

Closed benlings closed 1 year ago

benlings commented 1 year ago

Description

Using NavigationStackStore, with a destination view then presenting a third view using .navigationDestination(store:destination). When the third view is presented, SwiftUI goes into an infinite loop, consuming 100% CPU and increasing amount of memory.

Checklist

Expected behavior

The third view is pushed onto the navigation stack

Actual behavior

The app hangs with 100% CPU usage

Steps to reproduce

I've made a sample project that demonstrates the bug, along with demonstrations of the same functionality working correctly in plain SwiftUI. The TCA version is shown below.

In the sample project there are 3 tabs:

  1. Vanilla SwiftUI - using an array-based NavigationStack path
  2. VanillaStack - TCA, using an array-based NavigationStack path. Note: I had to extract NavigationDestinationView to stop this happening in this implementation
  3. TCA - TCA, using NavigationStackStore
struct ContentFeature: ReducerProtocol {
    struct State {
      var path = StackState<Path.State>()
    }
    public enum Action {
      case path(StackAction<Path.State, Path.Action>)
    }

    var body: some ReducerProtocolOf<Self> {
      Reduce { state, action in
        switch action {
        case .path:
          return .none
        }
      }
      .forEach(\.path, action: /Action.path) {
        Path()
      }
    }

    public struct Path: ReducerProtocol {
      public enum State {
        case int(IntFeature.State)
      }
      public enum Action {
        case int(IntFeature.Action)
      }
      public var body: some ReducerProtocolOf<Self> {
        Scope(state: /State.int, action: /Action.int) {
          IntFeature()
        }
      }
    }

  }

  struct ContentView: View {

    let store: StoreOf<ContentFeature>

    var body: some View {
      NavigationStackStore(store.scope(state: \.path, action: { .path($0) })) {
        VStack {
          NavigationLink("Nav link to 1", state: ContentFeature.Path.State.int(.init(num: 1)))
        }
        .padding()
      } destination: { store in
        DestinationSwitchView(store: store)
      }
    }
  }

  struct DestinationSwitchView: View {
    let store: StoreOf<ContentFeature.Path>
    var body: some View {
      SwitchStore(store) { state in
        switch state {
        case .int:
          CaseLet(
            state: /ContentFeature.Path.State.int,
            action: ContentFeature.Path.Action.int
          ) {
            IntView(store: $0)
          }
        }
      }
    }
  }

  struct IntFeature: ReducerProtocol {
    struct State: Equatable {
      var num: Int
      @PresentationState var dest: DestFeature.State? = nil
    }
    public enum Action {
      case showDest
      case dest(PresentationAction<DestFeature.Action>)
    }
    var body: some ReducerProtocolOf<Self> {
      Reduce { state, action in
        switch action {
        case .showDest:
          state.dest = .init()
          return .none

        case .dest:
          return .none
        }
      }
      .ifLet(\.$dest, action: /Action.dest) {
        DestFeature()
      }
    }
  }

  struct IntView: View {
    let store: StoreOf<IntFeature>

    var body: some View {
      WithViewStore(store, observe: { $0 }) { viewStore in
        VStack {
          Text("Number \(viewStore.num)")
          Button("Show 2") {
            viewStore.send(.showDest)
          }
        }
        .navigationDestination(
          store: store.scope(state: \.$dest, action: { .dest($0) })
        ) {
          DestView(store: $0)
        }
      }
    }
  }

  struct DestFeature: ReducerProtocol {
    struct State: Equatable {
    }
    enum Action {
    }
    var body: some ReducerProtocolOf<Self> {
      Reduce { state, action in
        switch action {
        }
      }
    }
  }

  struct DestView: View {
    let store: StoreOf<DestFeature>

    var body: some View {
      Text("Destination 2")
    }
  }

The Composable Architecture version information

0.55.1

Destination operating system

iOS 16.4

Xcode version information

Version 14.3.1 (14E300c)

Swift Compiler version information

swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)
Target: arm64-apple-macosx13.0
benlings commented 1 year ago

It looks like this can be fixed with the following patch to TCA:

diff --git a/Sources/ComposableArchitecture/SwiftUI/NavigationStackStore.swift b/Sources/ComposableArchitecture/SwiftUI/NavigationStackStore.swift
index c417abedb..2ca2fbe57 100644
--- a/Sources/ComposableArchitecture/SwiftUI/NavigationStackStore.swift
+++ b/Sources/ComposableArchitecture/SwiftUI/NavigationStackStore.swift
@@ -105,9 +105,7 @@ public struct NavigationStackStore<State, Action, Root: View, Destination: View>
       self.root
         .environment(\.navigationDestinationType, State.self)
         .navigationDestination(for: Component<State>.self) { component in
-          self.destination(component)
-            .environment(\.navigationDestinationType, State.self)
-            .id(component.id)
+          NavigationDestinationView(component: component, destination: self.destination)
         }
     }
   }
@@ -144,6 +142,16 @@ public struct _NavigationLinkStoreContent<State, Label: View>: View {
   }
 }

+private struct NavigationDestinationView<State, Destination: View>: View {
+  let component: Component<State>
+  let destination: (Component<State>) -> Destination
+  var body: some View {
+    self.destination(component)
+      .environment(\.navigationDestinationType, State.self)
+      .id(component.id)
+  }
+}
+
 @available(iOS 16, macOS 13, tvOS 16, watchOS 9, *)
 extension NavigationLink where Destination == Never {
   /// Creates a navigation link that presents the view corresponding to an element of
benlings commented 1 year ago

This workaround was based on this comment: https://github.com/pointfreeco/swift-composable-architecture/discussions/1842#discussioncomment-4708055