Skip to content

Commit fb6925e

Browse files
committed
fix(navigation): prevent duplicate Sheet key crash in single-route navigateTo
The single-route navigateTo(route) extension called navigate() directly without checking whether a sheet was already open. This allowed a second Sheet(TokenDiscovery) to land on the backstack during a dismiss animation, crashing SaveableStateProvider with 'Key was used multiple times'. - Single-route navigateTo now defers via pendingSheetDismiss when a sheet is already present, matching the multi-route overload behavior - Add Sheet dedup guard to the ClearAll navigate branch - Add async duplicate Sheet sanitization in AppNavHost as a safety net - Add 4 test cases covering the single-route dismiss-then-replace flow
1 parent ce56990 commit fb6925e

4 files changed

Lines changed: 106 additions & 1 deletion

File tree

apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/extensions/CodeNavigator.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,19 @@ fun CodeNavigator.navigateTo(route: NavKey, options: NavOptions = NavOptions())
1616
} else {
1717
route
1818
}
19-
navigate(destination, options)
19+
val needsSheet = destination is AppRoute.Main.Sheet
20+
val hasSheet = backStack.any { it is AppRoute.Main.Sheet }
21+
22+
if (hasSheet && needsSheet) {
23+
pendingSheetDismiss = {
24+
Snapshot.withMutableSnapshot {
25+
sheetGeneration++
26+
navigate(destination, options)
27+
}
28+
}
29+
} else {
30+
navigate(destination, options)
31+
}
2032
}
2133

2234
/**

apps/flipcash/shared/router/src/test/kotlin/com/flipcash/app/router/internal/NavigateToTest.kt

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,68 @@ class NavigateToTest {
277277
}
278278

279279
// endregion
280+
281+
// region Single-route navigateTo dismiss-then-replace
282+
283+
@Test
284+
fun `single-route navigateTo with existing sheet sets pendingSheetDismiss`() {
285+
val navigator = createNavigator(
286+
AppRoute.Main.Scanner,
287+
AppRoute.Main.Sheet(AppRoute.Sheets.Wallet),
288+
)
289+
290+
navigator.navigateTo(AppRoute.Sheets.TokenDiscovery, options = quietOptions)
291+
292+
assertNotNull(navigator.pendingSheetDismiss)
293+
// Backstack unchanged until the callback fires
294+
assertEquals(2, navigator.backStack.size)
295+
}
296+
297+
@Test
298+
fun `single-route navigateTo callback navigates to new sheet after dismiss`() {
299+
val navigator = createNavigator(
300+
AppRoute.Main.Scanner,
301+
AppRoute.Main.Sheet(AppRoute.Sheets.Wallet),
302+
)
303+
304+
navigator.navigateTo(AppRoute.Sheets.TokenDiscovery, options = quietOptions)
305+
306+
// Simulate dismiss: remove old sheet entry, then callback fires
307+
navigator.backStack.removeAt(navigator.backStack.lastIndex)
308+
navigator.pendingSheetDismiss!!.invoke()
309+
310+
val last = navigator.backStack.last()
311+
assertIs<AppRoute.Main.Sheet>(last)
312+
assertEquals(AppRoute.Sheets.TokenDiscovery, last.initialRoute)
313+
}
314+
315+
@Test
316+
fun `single-route navigateTo increments sheetGeneration on dismiss-replace`() {
317+
val navigator = createNavigator(
318+
AppRoute.Main.Scanner,
319+
AppRoute.Main.Sheet(AppRoute.Sheets.Wallet),
320+
)
321+
val initialGeneration = navigator.sheetGeneration
322+
323+
navigator.navigateTo(AppRoute.Sheets.TokenDiscovery, options = quietOptions)
324+
325+
// Simulate dismiss
326+
navigator.backStack.removeAt(navigator.backStack.lastIndex)
327+
navigator.pendingSheetDismiss!!.invoke()
328+
329+
assertEquals(initialGeneration + 1, navigator.sheetGeneration)
330+
}
331+
332+
@Test
333+
fun `single-route navigateTo without existing sheet navigates directly`() {
334+
val navigator = createNavigator(AppRoute.Main.Scanner)
335+
336+
navigator.navigateTo(AppRoute.Sheets.TokenDiscovery, options = quietOptions)
337+
338+
assertNull(navigator.pendingSheetDismiss)
339+
assertEquals(2, navigator.backStack.size)
340+
assertIs<AppRoute.Main.Sheet>(navigator.backStack.last())
341+
}
342+
343+
// endregion
280344
}

ui/navigation/src/main/kotlin/com/getcode/navigation/AppNavHost.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import androidx.navigation3.scene.OverlayScene
1717
import androidx.compose.material3.ExperimentalMaterial3Api
1818
import androidx.compose.runtime.Composable
1919
import androidx.compose.runtime.LaunchedEffect
20+
import androidx.compose.runtime.snapshotFlow
21+
import androidx.compose.runtime.snapshots.Snapshot
2022
import androidx.compose.ui.graphics.Color
2123
import androidx.compose.ui.graphics.luminance
2224
import androidx.compose.ui.graphics.toArgb
@@ -35,6 +37,7 @@ import com.getcode.navigation.decorators.rememberNavResultScopeEntryDecorator
3537
import com.getcode.navigation.results.NavResultStateRegistry
3638
import com.getcode.navigation.results.rememberNavResultStateRegistry
3739
import com.getcode.theme.CodeTheme
40+
import timber.log.Timber
3841

3942
@OptIn(ExperimentalMaterial3Api::class)
4043
@Composable
@@ -60,6 +63,29 @@ fun AppNavHost(
6063
) {
6164
ChangeSystemBarsTheme(CodeTheme.colors.background.luminance() < 0.5f)
6265

66+
// Safety net: async duplicate Sheet sanitization.
67+
// Cannot prevent a same-frame crash, but cleans up residual duplicates
68+
// from unforeseen race conditions before the next frame renders.
69+
LaunchedEffect(navigator.backStack) {
70+
snapshotFlow { navigator.backStack.toList() }
71+
.collect { stack ->
72+
val seen = mutableSetOf<String>()
73+
val toRemove = mutableListOf<Int>()
74+
for (i in stack.lastIndex downTo 0) {
75+
val entry = stack[i]
76+
if (entry is Sheet && !seen.add(entry.toString())) {
77+
toRemove.add(i)
78+
}
79+
}
80+
if (toRemove.isNotEmpty()) {
81+
Timber.w("Duplicate Sheet keys detected, sanitizing backstack")
82+
Snapshot.withMutableSnapshot {
83+
toRemove.forEach { navigator.backStack.removeAt(it) }
84+
}
85+
}
86+
}
87+
}
88+
6389
NavDisplay(
6490
backStack = navigator.backStack,
6591
onBack = onBack ?: {

ui/navigation/src/main/kotlin/com/getcode/navigation/core/CodeNavigator.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ data class CodeNavigator(
9999
NavOptions.PopUpTo.ClearAll -> {
100100
Snapshot.withMutableSnapshot {
101101
if (currentRouteKey != route) {
102+
if (route is Sheet) {
103+
backStack.removeAll { it is Sheet && it.toString() == route.toString() }
104+
}
102105
backStack.add(route)
103106
// Remove all entries before the new one
104107
while (backStack.size > 1) {

0 commit comments

Comments
 (0)