Fix remaining retain cycles, allow coordinators to be weakly referenced and make sure the NavigationStackCoordinator cleans up after itself

ElementX/Sources/Application/CoordinatorProtocol.swift

@@ -17,7 +17,7 @@
 import SwiftUI
 
 @MainActor
-protocol CoordinatorProtocol {
+protocol CoordinatorProtocol: AnyObject {
     func start()
     func stop()
     func toPresentable() -> AnyView

ElementX/Sources/Application/Navigation/NavigationCoordinators.swift

@@ -213,14 +213,14 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
     ///
     /// For added complexity, the NavigationSplitCoordinator has an internal compact layout NavigationStack for which we need to manually nil things again
     private func releaseAllCoordinatorReferences() {
-        sidebarModule?.coordinator = nil
+        sidebarModule?.tearDown()
-        detailModule?.coordinator = nil
+        detailModule?.tearDown()
-        sheetModule?.coordinator = nil
+        sheetModule?.tearDown()
-        fullScreenCoverModule?.coordinator = nil
+        fullScreenCoverModule?.tearDown()
         
-        compactLayoutRootModule?.coordinator = nil
+        compactLayoutRootModule?.tearDown()
         compactLayoutStackModules.forEach { module in
-            module.coordinator = nil
+            module.tearDown()
         }
     }
     
@@ -594,6 +594,19 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
             .presentationDetents(presentationDetents))
     }
     
+    /// The NavigationStack has a tendency to hold on to path items for longer than needed. We work around that by manually nilling the coordinator
+    /// when a NavigationModule is dismissed. As the NavigationModule is just a wrapper multiple instances of it continuing living is of no consequence
+    /// https://stackoverflow.com/questions/73885353/found-a-strange-behaviour-of-state-when-combined-to-the-new-navigation-stack/
+    func stop() {
+        rootModule?.tearDown()
+        sheetModule?.tearDown()
+        fullScreenCoverModule?.tearDown()
+        
+        stackModules.forEach { module in
+            module.tearDown()
+        }
+    }
+    
     // MARK: - CustomStringConvertible
     
     var description: String {

ElementX/Sources/Application/Navigation/NavigationModule.swift

@@ -27,7 +27,7 @@ class NavigationModule: Identifiable, Hashable {
     /// As the NavigationModule is just a wrapper multiple instances of it continuing living is of no consequence
     /// https://stackoverflow.com/questions/73885353/found-a-strange-behaviour-of-state-when-combined-to-the-new-navigation-stack/
     var coordinator: (any CoordinatorProtocol)?
-    let dismissalCallback: (() -> Void)?
+    var dismissalCallback: (() -> Void)?
     
     init(_ coordinator: any CoordinatorProtocol, dismissalCallback: (() -> Void)? = nil) {
         self.coordinator = coordinator
@@ -38,6 +38,7 @@ class NavigationModule: Identifiable, Hashable {
         coordinator?.stop()
         dismissalCallback?()
         coordinator = nil
+        dismissalCallback = nil
     }
     
     nonisolated static func == (lhs: NavigationModule, rhs: NavigationModule) -> Bool {

ElementX/Sources/Other/UserNotifications/MockUserNotificationController.swift

@@ -16,7 +16,7 @@
 
 import Foundation
 
-struct MockUserNotificationController: UserNotificationControllerProtocol {
+class MockUserNotificationController: UserNotificationControllerProtocol {
     func submitNotification(_ notification: UserNotification) { }
     
     func retractNotificationWithId(_ id: String) { }

ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift

@@ -23,7 +23,7 @@ enum BugReportCoordinatorResult {
 
 struct BugReportCoordinatorParameters {
     let bugReportService: BugReportServiceProtocol
-    let userNotificationController: UserNotificationControllerProtocol
+    weak var userNotificationController: UserNotificationControllerProtocol?
     let screenshot: UIImage?
     let isModallyPresented: Bool
 }
@@ -76,17 +76,17 @@ final class BugReportCoordinator: CoordinatorProtocol {
     static let loadingIndicatorIdentifier = "BugReportLoading"
     
     private func startLoading(label: String = ElementL10n.loading) {
-        parameters.userNotificationController.submitNotification(UserNotification(id: Self.loadingIndicatorIdentifier,
+        parameters.userNotificationController?.submitNotification(UserNotification(id: Self.loadingIndicatorIdentifier,
-                                                                                  type: .modal,
+                                                                                   type: .modal,
-                                                                                  title: label,
+                                                                                   title: label,
-                                                                                  persistent: true))
+                                                                                   persistent: true))
     }
     
     private func stopLoading() {
-        parameters.userNotificationController.retractNotificationWithId(Self.loadingIndicatorIdentifier)
+        parameters.userNotificationController?.retractNotificationWithId(Self.loadingIndicatorIdentifier)
     }
     
     private func showError(label: String) {
-        parameters.userNotificationController.submitNotification(UserNotification(title: label))
+        parameters.userNotificationController?.submitNotification(UserNotification(title: label))
     }
 }

ElementX/Sources/Screens/Other/ActivityCoordinator.swift

@@ -16,9 +16,13 @@
 
 import SwiftUI
 
-struct ActivityCoordinator: CoordinatorProtocol {
+class ActivityCoordinator: CoordinatorProtocol {
     let items: [Any]
     
+    init(items: [Any]) {
+        self.items = items
+    }
+    
     func toPresentable() -> AnyView {
         AnyView(UIActivityViewControllerWrapper(activityItems: items)
             .presentationDetents([.medium])

ElementX/Sources/Screens/Other/InviteFriendsCoordinator.swift

@@ -16,9 +16,13 @@
 
 import SwiftUI
 
-struct InviteFriendsCoordinator: CoordinatorProtocol {
+class InviteFriendsCoordinator: CoordinatorProtocol {
     let userId: String
     
+    init(userId: String) {
+        self.userId = userId
+    }
+    
     func toPresentable() -> AnyView {
         guard let permalink = try? PermalinkBuilder.permalinkTo(userIdentifier: userId).absoluteString else {
             return AnyView(EmptyView())

ElementX/Sources/Screens/Other/SplashScreenCoordinator.swift

@@ -16,7 +16,7 @@
 
 import SwiftUI
 
-struct SplashScreenCoordinator: CoordinatorProtocol {
+class SplashScreenCoordinator: CoordinatorProtocol {
     func toPresentable() -> AnyView {
         AnyView(
             ZStack {

ElementX/Sources/Screens/Settings/SettingsScreenCoordinator.swift

@@ -17,8 +17,8 @@
 import SwiftUI
 
 struct SettingsScreenCoordinatorParameters {
-    let navigationStackCoordinator: NavigationStackCoordinator
+    weak var navigationStackCoordinator: NavigationStackCoordinator?
-    let userNotificationController: UserNotificationControllerProtocol
+    weak var userNotificationController: UserNotificationControllerProtocol?
     let userSession: UserSessionProtocol
     let bugReportService: BugReportServiceProtocol
 }
@@ -88,10 +88,10 @@ final class SettingsScreenCoordinator: CoordinatorProtocol {
                 break
             }
             
-            self?.parameters.navigationStackCoordinator.pop()
+            self?.parameters.navigationStackCoordinator?.pop()
         }
         
-        parameters.navigationStackCoordinator.push(coordinator)
+        parameters.navigationStackCoordinator?.push(coordinator)
     }
     
     private func verifySession() {
@@ -103,15 +103,15 @@ final class SettingsScreenCoordinator: CoordinatorProtocol {
         let coordinator = SessionVerificationCoordinator(parameters: verificationParameters)
         
         coordinator.callback = { [weak self] in
-            self?.parameters.navigationStackCoordinator.setSheetCoordinator(nil)
+            self?.parameters.navigationStackCoordinator?.setSheetCoordinator(nil)
         }
         
-        parameters.navigationStackCoordinator.setSheetCoordinator(coordinator) { [weak self] in
+        parameters.navigationStackCoordinator?.setSheetCoordinator(coordinator) { [weak self] in
-            self?.parameters.navigationStackCoordinator.setSheetCoordinator(nil)
+            self?.parameters.navigationStackCoordinator?.setSheetCoordinator(nil)
         }
     }
 
     private func showSuccess(label: String) {
-        parameters.userNotificationController.submitNotification(UserNotification(title: label))
+        parameters.userNotificationController?.submitNotification(UserNotification(title: label))
     }
 }

ElementX/Sources/UITests/UITestsAppCoordinator.swift

@@ -119,7 +119,7 @@ class MockScreen: Identifiable {
         case .settings:
             let navigationStackCoordinator = NavigationStackCoordinator()
             let coordinator = SettingsScreenCoordinator(parameters: .init(navigationStackCoordinator: navigationStackCoordinator,
-                                                                          userNotificationController: MockUserNotificationController(),
+                                                                          userNotificationController: nil,
                                                                           userSession: MockUserSession(clientProxy: MockClientProxy(userID: "@mock:client.com"),
                                                                                                        mediaProvider: MockMediaProvider()),
                                                                           bugReportService: MockBugReportService()))
@@ -128,7 +128,7 @@ class MockScreen: Identifiable {
         case .bugReport:
             let navigationStackCoordinator = NavigationStackCoordinator()
             let coordinator = BugReportCoordinator(parameters: .init(bugReportService: MockBugReportService(),
-                                                                     userNotificationController: MockUserNotificationController(),
+                                                                     userNotificationController: nil,
                                                                      screenshot: nil,
                                                                      isModallyPresented: true))
             navigationStackCoordinator.setRootCoordinator(coordinator)
@@ -136,7 +136,7 @@ class MockScreen: Identifiable {
         case .bugReportWithScreenshot:
             let navigationStackCoordinator = NavigationStackCoordinator()
             let coordinator = BugReportCoordinator(parameters: .init(bugReportService: MockBugReportService(),
-                                                                     userNotificationController: MockUserNotificationController(),
+                                                                     userNotificationController: nil,
                                                                      screenshot: Asset.Images.appLogo.image,
                                                                      isModallyPresented: false))
             navigationStackCoordinator.setRootCoordinator(coordinator)

ElementX/Sources/UITests/UITestsRootCoordinator.swift

@@ -16,18 +16,23 @@
 
 import SwiftUI
 
-struct UITestsRootCoordinator: CoordinatorProtocol {
+class UITestsRootCoordinator: CoordinatorProtocol {
     let mockScreens: [MockScreen]
     var selectionCallback: ((UITestScreenIdentifier) -> Void)?
     
+    init(mockScreens: [MockScreen], selectionCallback: ((UITestScreenIdentifier) -> Void)? = nil) {
+        self.mockScreens = mockScreens
+        self.selectionCallback = selectionCallback
+    }
+    
     func toPresentable() -> AnyView {
         AnyView(body)
     }
     
     private var body: some View {
         List(mockScreens) { coordinator in
-            Button(coordinator.id.description) {
+            Button(coordinator.id.description) { [weak self] in
-                selectionCallback?(coordinator.id)
+                self?.selectionCallback?(coordinator.id)
             }
             .accessibilityIdentifier(coordinator.id.rawValue)
         }

UnitTests/Sources/NavigationRootCoordinatorTests.swift

@@ -67,6 +67,6 @@ class NavigationRootCoordinatorTests: XCTestCase {
     }
 }
 
-private struct SomeTestCoordinator: CoordinatorProtocol {
+private class SomeTestCoordinator: CoordinatorProtocol {
     let id = UUID()
 }

UnitTests/Sources/NavigationSplitCoordinatorTests.swift

@@ -269,6 +269,6 @@ class NavigationSplitCoordinatorTests: XCTestCase {
     }
 }
 
-private struct SomeTestCoordinator: CoordinatorProtocol {
+private class SomeTestCoordinator: CoordinatorProtocol {
     let id = UUID()
 }

UnitTests/Sources/NavigationStackCoordinatorTests.swift

@@ -212,6 +212,6 @@ class NavigationStackCoordinatorTests: XCTestCase {
     }
 }
 
-private struct SomeTestCoordinator: CoordinatorProtocol {
+private class SomeTestCoordinator: CoordinatorProtocol {
     let id = UUID()
 }