Fix possible desync of navigation if listener is null (#9)

* Fix possible desync of navigation if listener is null * Handle push return value on iOS * Add return value for push * api dump --------- Co-authored-by: Hugo Lefrancois <[email protected]>
mirego · Apr 17, 2024 · 0934317 · 0934317
1 parent fd86091
commit 0934317
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 12 deletions.
diff --git a/navigation/common/api/navigation.api b/navigation/common/api/navigation.api
@@ -32,7 +32,7 @@ public abstract class com/mirego/pilot/navigation/PilotNavigationListener {
 	public fun <init> ()V
 	public abstract fun pop ()V
 	public abstract fun popTo (Lcom/mirego/pilot/navigation/PilotNavigationRoute;Z)V
-	public abstract fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)V
+	public abstract fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)Z
 }
 
 public abstract class com/mirego/pilot/navigation/PilotNavigationManager {
@@ -70,7 +70,7 @@ public class com/mirego/pilot/navigation/compose/PilotNavControllerNavigationLis
 	public fun <init> (Landroidx/navigation/NavController;)V
 	public fun pop ()V
 	public fun popTo (Lcom/mirego/pilot/navigation/PilotNavigationRoute;Z)V
-	public fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)V
+	public fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)Z
 }
 
 public final class com/mirego/pilot/navigation/compose/PilotNavigationHelpersKt {

diff --git a/...idMain/kotlin/com/mirego/pilot/navigation/compose/PilotNavControllerNavigationListener.kt b/...idMain/kotlin/com/mirego/pilot/navigation/compose/PilotNavControllerNavigationListener.kt
@@ -8,8 +8,9 @@ public open class PilotNavControllerNavigationListener<ROUTE : PilotNavigationRo
     private val navController: NavController,
 ) : PilotNavigationListener<ROUTE>() {
 
-    override fun push(route: ROUTE) {
+    override fun push(route: ROUTE): Boolean {
         navController.navigate(route.navRoute)
+        return true
     }
 
     override fun pop() {

diff --git a/...common/src/commonMain/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManager.kt b/...common/src/commonMain/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManager.kt
@@ -23,9 +23,11 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
                 parentNavigationManager.push(route, locally = locally)
                 return@launch
             }
-
-            internalRouteList.add(route)
-            listener?.push(route)
+            listener?.let {
+                if (it.push(route)) {
+                    internalRouteList.add(route)
+                }
+            }
         }
     }
 
@@ -75,9 +77,14 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
 
             if (index != -1 && internalRouteList.isNotEmpty()) {
                 val effectiveIndex = index + if (inclusive) 0 else 1
-                internalRouteList.removeAll(internalRouteList.takeLast(internalRouteList.size - effectiveIndex))
+                val elementsToRemove = internalRouteList.takeLast(internalRouteList.size - effectiveIndex)
                 if (callListener) {
-                    listener?.popTo(navigationItem, inclusive = inclusive)
+                    listener?.let {
+                        internalRouteList.removeAll(elementsToRemove)
+                        listener?.popTo(navigationItem, inclusive = inclusive)
+                    }
+                } else {
+                    internalRouteList.removeAll(elementsToRemove)
                 }
             }
         }

diff --git a/...gation/common/src/commonMain/kotlin/com/mirego/pilot/navigation/PilotNavigationManager.kt b/...gation/common/src/commonMain/kotlin/com/mirego/pilot/navigation/PilotNavigationManager.kt
@@ -21,7 +21,7 @@ public abstract class PilotNavigationManager<ROUTE : PilotNavigationRoute, ACTIO
 }
 
 public abstract class PilotNavigationListener<ROUTE : PilotNavigationRoute> {
-    public abstract fun push(route: ROUTE)
+    public abstract fun push(route: ROUTE): Boolean
     public abstract fun pop()
     public abstract fun popTo(route: ROUTE, inclusive: Boolean)
 }

diff --git a/navigation/ios/NavigationState.swift b/navigation/ios/NavigationState.swift
@@ -46,18 +46,18 @@ class NavigationState<
         actionListener.startListening()
     }
 
-    override func push(route: PilotNavigationRoute) {
+    override func push(route: PilotNavigationRoute) -> Bool {
         guard let buildNavigation else { fatalError("buildNavigation not set")}
         guard let route = route as? Route else { fatalError("Invalid route type")}
         guard let navigation = buildNavigation(currentStack(), route) else {
-            navigationManager?.popToId(uniqueId: route.uniqueId, inclusive: true)
-            return
+            return false
         }
 
         dedounceNavigation { [weak self] in
             guard let self else { return }
             top().child = NavigationState(navigation: navigation, route: route)
         }
+        return true
     }
 
     override func popTo(route: PilotNavigationRoute, inclusive: Bool) {