Fix getRouteWithArgs when an argument is not registered as route type…

…d argument (#1634)

Fix untyped navigation route arguments.

Before a route was empty if an app uses:
```kotlin
route("screen?id={id}") {
  it.arguments.getString("id")
}
```

navigation/navigation-runtime/src/jsTest/kotlin/androidx/navigation/BrowserHistoryTest.kt

@@ -42,6 +42,9 @@ class BrowserHistoryTest {
                     }
                 }
                 test("screen_5")
+                test("screen_6/{pathId}?q={queryId}") {
+                    argument("pathId") { type = NavType.IntType }
+                }
             }
         }
 
@@ -83,6 +86,15 @@ class BrowserHistoryTest {
             .inOrder()
         assertThat(window.location.toString()).isEqualTo("$appAddress#screen_2")
 
+        navController.navigate("screen_6/123?q=456")
+        advanceUntilIdle()
+
+        assertThat(window.history.length).isEqualTo(4)
+        assertThat(window.history.state.toString().lines())
+            .containsExactly("screen_5", "screen_2", "screen_6/123?q=456")
+            .inOrder()
+        assertThat(window.location.toString()).isEqualTo("$appAddress#screen_6/123?q=456")
+
         bind.cancel()
     }
 
@@ -112,7 +124,19 @@ class BrowserHistoryTest {
         navController.navigate("screen_2")
         advanceUntilIdle()
 
-        assertThat(window.history.length).isEqualTo(5)
+        navController.navigate("screen_6/123")
+        advanceUntilIdle()
+
+        assertThat(window.history.length).isEqualTo(6)
+        assertThat(window.history.state.toString().lines())
+            .containsExactly("screen_5", "screen_2", "screen_6/123?q=")
+            .inOrder()
+        assertThat(window.location.toString()).isEqualTo("$appAddress#screen_6/123?q=")
+
+        window.history.back()
+        waitHistoryStateUpdate()
+
+        assertThat(window.history.length).isEqualTo(6)
         assertThat(window.history.state.toString().lines())
             .containsExactly("screen_5", "screen_2")
             .inOrder()
@@ -121,7 +145,7 @@ class BrowserHistoryTest {
         window.history.back()
         waitHistoryStateUpdate()
 
-        assertThat(window.history.length).isEqualTo(5)
+        assertThat(window.history.length).isEqualTo(6)
         assertThat(window.history.state.toString().lines())
             .containsExactly("screen_5")
             .inOrder()
@@ -130,7 +154,7 @@ class BrowserHistoryTest {
         window.history.back()
         waitHistoryStateUpdate()
 
-        assertThat(window.history.length).isEqualTo(5)
+        assertThat(window.history.length).isEqualTo(6)
         assertThat(window.history.state.toString().lines())
             .containsExactly("screen_1", "screen_2", "screen_4")
             .inOrder()
@@ -139,7 +163,7 @@ class BrowserHistoryTest {
         window.history.back()
         waitHistoryStateUpdate()
 
-        assertThat(window.history.length).isEqualTo(5)
+        assertThat(window.history.length).isEqualTo(6)
         assertThat(window.history.state.toString().lines())
             .containsExactly("screen_1", "screen_2")
             .inOrder()

navigation/navigation-runtime/src/webMain/kotlin/androidx/navigation/BrowserHistory.kt

@@ -37,12 +37,15 @@ internal suspend fun BrowserWindow.bindToNavigation(
     coroutineScope {
         val localWindow = this@bindToNavigation
         val appAddress = with(localWindow.location) { origin + pathname }
-        var initState = true
-        var updateState = true
 
         launch {
             localWindow.popStateEvents().collect { event ->
-                val state = event.state.toString()
+                val state = event.state
+
+                if (state == null) {
+                    //if user manually put a new address or open a new page, then there is no state
+                    return@collect
+                }
 
                 val restoredRoutes = state.lines()
                 val currentBackStack = navController.currentBackStack.value
@@ -59,9 +62,6 @@ internal suspend fun BrowserWindow.bindToNavigation(
                     }
                 }
 
-                //don't handle next navigation calls
-                updateState = false
-
                 if (commonTail == -1) {
                     //clear full stack
                     currentRoutes.firstOrNull()?.let { root ->
@@ -92,16 +92,27 @@ internal suspend fun BrowserWindow.bindToNavigation(
                 val newUri = appAddress + getBackStackEntryRoute(entries.last())
                 val state = routes.joinToString("\n")
 
+                val currentState = localWindow.history.state
+                when (currentState) {
+                    null -> {
+                        //user manually put a new address or open a new page,
+                        // we need to save the current state in the browser history
+                        localWindow.history.replaceState(state, "", newUri)
+                    }
 
-                if (updateState) {
-                    if (initState) {
+                    state -> {
+                        //this was a restoration of the state (back/forward browser navigation)
+                        //the callback came from the popStateEvents
+                        //the browser state is equal the app state, but we need to update shown uri
                         localWindow.history.replaceState(state, "", newUri)
-                        initState = false
-                    } else {
+                    }
+
+                    else -> {
+                        //the navigation happened in the compose app,
+                        // we need to push the new state to the browser history
                         localWindow.history.pushState(state, "", newUri)
                     }
                 }
-                updateState = true
             }
         }
     }
@@ -123,22 +134,26 @@ private fun BrowserWindow.popStateEvents(): Flow<BrowserPopStateEvent> = callbac
     }
 }
 
-private val argPlaceholder = Regex("""\{*.\}""")
+private val argPlaceholder = Regex("""\{.*?\}""")
 internal fun NavBackStackEntry.getRouteWithArgs(): String? {
     val entry = this
     val route = entry.destination.route ?: return null
     if (!route.contains(argPlaceholder)) return route
     val args = entry.arguments ?: Bundle()
-    val nameToValue = entry.destination.arguments.map { (name, arg) ->
-        val serializedTypeValue = arg.type.serializeAsValue(arg.type[args, name])
-        name to serializedTypeValue
+    val nameToTypedValue = entry.destination.arguments.mapValues { (name, arg) ->
+        arg.type.serializeAsValue(arg.type[args, name])
     }
 
-    val routeWithFilledArgs =
-        nameToValue.fold(initial = route) { acc, (argumentName: String, value: String) ->
-            acc.replace("{$argumentName}", value)
-        }
-    return routeWithFilledArgs.takeIf { !it.contains(argPlaceholder) }
+    val routeWithFilledArgs = route.replace(argPlaceholder) { match ->
+        val key = match.value.trim('{', '}')
+        nameToTypedValue[key] ?: if (args.containsKey(key)) {
+            //untyped args stored as strings
+            //see: androidx.navigation.NavDeepLink.parseArgument
+            args.getString(key)!!
+        } else ""
+    }
+
+    return routeWithFilledArgs
 }
 
 internal external interface BrowserLocation {
@@ -147,6 +162,7 @@ internal external interface BrowserLocation {
 }
 
 internal external interface BrowserHistory {
+    val state: String?
     fun pushState(data: String?, title: String, url: String?)
     fun replaceState(data: String?, title: String, url: String?)
 }