Fixed web history rewriting incorrectly when started via a link

Author: arkivanov

decompose/src/webMain/kotlin/com/arkivanov/decompose/router/webhistory/WebHistoryNavigation.kt

@@ -60,15 +60,15 @@ internal fun <T : Any> enableWebHistory(navigation: WebNavigation<T>, browserHis
                 browserHistory.replaceState(nodes)
             }
         },
-        onRewrite = { oldSize, newHistory ->
-            if (oldSize > 1) {
+        onRewrite = { newHistory ->
+            val currentIndex = browserHistory.currentIndex()
+            if (currentIndex > 0) {
                 browserHistory.setOnPopStateListener {
                     browserHistory.setOnPopStateListener(::onPopState)
                     browserHistory.replaceState(newHistory.first())
                     newHistory.drop(1).forEach(browserHistory::pushState)
                 }
-
-                browserHistory.go(-oldSize + 1)
+                browserHistory.go(-currentIndex)
             } else {
                 browserHistory.replaceState(newHistory.first())
                 newHistory.drop(1).forEach(browserHistory::pushState)
@@ -135,7 +135,7 @@ private fun <T : Any> WebNavigation<T>.subscribe(
     isEnabled: () -> Boolean,
     onPush: (List<NodeHistory<T>>) -> Unit,
     onPop: (count: Int, NodeHistory<T>) -> Unit,
-    onRewrite: (oldSize: Int, newHistory: List<NodeHistory<T>>) -> Unit,
+    onRewrite: (newHistory: List<NodeHistory<T>>) -> Unit,
     onUpdateUrl: (NodeHistory<T>) -> Unit,
 ): Cancellation {
     var activeChildCancellation: Cancellation? = null
@@ -164,11 +164,8 @@ private fun <T : Any> WebNavigation<T>.subscribe(
                 onPop = { count, childNodes ->
                     onPop(count, inactiveNodes + nodeOf(item = activeItem, children = childNodes))
                 },
-                onRewrite = { oldSize, childHistory ->
-                    onRewrite(
-                        oldSize,
-                        childHistory.map { childNodes -> inactiveNodes + nodeOf(item = activeItem, children = childNodes) },
-                    )
+                onRewrite = { childHistory ->
+                    onRewrite(childHistory.map { childNodes -> inactiveNodes + nodeOf(item = activeItem, children = childNodes) })
                 },
                 onUpdateUrl = { childNodes ->
                     onUpdateUrl(inactiveNodes + nodeOf(item = activeItem, children = childNodes))
@@ -182,7 +179,7 @@ private fun <T : Any> WebNavigation<T>.onHistoryChanged(
     oldHistory: List<HistoryItem<T>>,
     onPush: (List<NodeHistory<T>>) -> Unit,
     onPop: (count: Int, NodeHistory<T>) -> Unit,
-    onRewrite: (oldSize: Int, newHistory: List<NodeHistory<T>>) -> Unit,
+    onRewrite: (newHistory: List<NodeHistory<T>>) -> Unit,
     onUpdateUrl: (NodeHistory<T>) -> Unit,
 ) {
     val newKeys = newHistory.map { it.key }
@@ -226,9 +223,7 @@ private fun <T : Any> WebNavigation<T>.onHistoryChanged(
                 previousNodes += itemHistory.last()
             }
 
-            val oldPaths = oldHistory.flatMap(::historyOf)
-
-            onRewrite(oldPaths.size, historyChange)
+            onRewrite(historyChange)
         }
     }
 }

decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/TestBrowserHistory.kt

@@ -1,6 +1,7 @@
 package com.arkivanov.decompose.router.webhistory
 
 import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 
 class TestBrowserHistory : BrowserHistory {
 
@@ -13,7 +14,9 @@ class TestBrowserHistory : BrowserHistory {
 
     override fun go(delta: Int) {
         scheduleOperation {
+            val oldIndex = index
             index += delta
+            assertTrue(index in stack.indices, "Invalid go operation: delta=$delta, index=$oldIndex, range=${stack.indices}")
             onPopStateListener?.invoke(stack[index].data)
         }
     }

decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/TestWebNavigation.kt

@@ -6,6 +6,7 @@ import com.arkivanov.decompose.value.Value
 import kotlinx.serialization.KSerializer
 import kotlinx.serialization.builtins.serializer
 import kotlin.test.assertContentEquals
+import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 

decompose/src/webTest/kotlin/com/arkivanov/decompose/router/webhistory/WebHistoryNavigationTest.kt

@@ -779,6 +779,25 @@ class WebHistoryNavigationTest {
         assertHistory(nav = nav, urls = listOf("/1/1/1", "/1/1/2"))
     }
 
+    @Test
+    fun GIVEN_created_one_root_and_two_children_WHEN_root_replaced_with_with_one_child_THEN_one_item_in_history() {
+        val nav =
+            TestWebNavigation(initialHistory = listOf(1)) { cfg ->
+                when (cfg) {
+                    1 ->  TestWebNavigation(initialHistory = listOf(12, 13))
+                    2 ->  TestWebNavigation(initialHistory = listOf(22))
+                    else -> null
+                }
+            }
+
+        enableWebHistory(nav, history)
+
+        nav.navigate(listOf(2))
+        history.runPendingOperations()
+
+        assertHistory(nav = nav, urls = listOf("/2/22"))
+    }
+
     private fun assertHistory(nav: TestWebNavigation, urls: List<String>, index: Int = urls.lastIndex) {
         history.assertStack(urls = urls, index = index)
         nav.assertHistory(urls = urls.slice(0..index))