Skip to content

Commit

Permalink
Remove compositionHash usage due duplication bug (#7)
Browse files Browse the repository at this point in the history
• fix multiple view models in same destination leads to crush
• fix multiple navControllers in same destination uses same DataStorage
  • Loading branch information
vkatz authored Feb 29, 2024
1 parent 5fd8db0 commit 8bdbca8
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 48 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package content.examples

import androidx.compose.foundation.border
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.MaterialTheme
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import com.composegears.tiamat.Navigation
Expand All @@ -13,14 +14,11 @@ import content.examples.common.SimpleScreen

val NestedNavigationRoot by navDestination<Unit> {
SimpleScreen("Nested navigation") {
Box(
Modifier
.padding(32.dp)
.border(4.dp, MaterialTheme.colorScheme.onSurface)
.padding(4.dp)
Column(
horizontalAlignment = Alignment.CenterHorizontally,
modifier = Modifier.padding(16.dp)
) {
val nestedNavController = rememberNavController(
key = "nestedNavController",
val nestedNavController1 = rememberNavController(
startDestination = SimpleForwardBackRoot,
destinations = arrayOf(
SimpleForwardBackRoot,
Expand All @@ -29,7 +27,31 @@ val NestedNavigationRoot by navDestination<Unit> {
SimpleForwardBackRootScreen3,
)
)
Navigation(nestedNavController)
val nestedNavController2 = rememberNavController(
startDestination = SimpleForwardBackRoot,
destinations = arrayOf(
SimpleForwardBackRoot,
SimpleForwardBackRootScreen1,
SimpleForwardBackRootScreen2,
SimpleForwardBackRootScreen3,
)
)
Navigation(
navController = nestedNavController1,
modifier = Modifier
.weight(1f)
.padding(8.dp)
.border(4.dp, MaterialTheme.colorScheme.onSurface)
.padding(4.dp)
)
Navigation(
navController = nestedNavController2,
modifier = Modifier
.weight(1f)
.padding(8.dp)
.border(4.dp, MaterialTheme.colorScheme.onSurface)
.padding(4.dp)
)
}
}
}
2 changes: 1 addition & 1 deletion library/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ plugins {
}

val libName = "io.github.composegears"
val libVersion = "1.0.0"
val libVersion = "1.0.1"

group = libName
version = libVersion
Expand Down
40 changes: 21 additions & 19 deletions library/src/commonMain/kotlin/com/composegears/tiamat/Compose.kt
Original file line number Diff line number Diff line change
Expand Up @@ -67,26 +67,17 @@ fun rememberNavController(
destinations: Array<NavDestination<*>>
): NavController {
val parent = LocalNavController.current
val parentDataStorage = LocalDataStore.current
val navDataStore =
if (parentDataStorage == null) rootDataStore()
else {
val storageKey = "DataStore#" + currentCompositeKeyHash.toString(16)
parentDataStorage.data.getOrPut(storageKey) { DataStorage() } as DataStorage
}

val parentDataStorage = LocalDataStore.current ?: rootDataStore()
fun createNavController(state: Map<String, Any?>?) =
NavController(
parent = parent,
key = key,
storageMode = storageMode ?: parent?.storageMode ?: ResetOnDataLoss,
startDestination = startDestination,
savedState = state,
dataStorage = navDataStore,
destinations = destinations
).apply {
if (storageMode == ResetOnDataLoss && navDataStore.data.isEmpty()) reset()
else restoreFromSavedState()
restoreState(parentDataStorage)
}
return rememberSaveable(
saver = Saver(
Expand Down Expand Up @@ -214,7 +205,7 @@ fun Navigation(
CompositionLocalProvider(
LocalSaveableStateRegistry provides saveRegistry,
LocalDataStore provides entryStorage,
LocalNavController provides navController
LocalNavController provides navController,
) {
DestinationContent(it.destination)
}
Expand Down Expand Up @@ -272,26 +263,37 @@ fun <Args> NavDestinationScope<Args>.navArgsOrNull(): Args? {
@Suppress("UNCHECKED_CAST", "CastToNullableType", "UnusedReceiverParameter")
fun <Result> NavDestinationScope<*>.navResult(): Result? {
val store = LocalDataStore.current ?: error("Store not bound")
val result = remember { store.data[KEY_NAV_RESULT] as Result? }
return result
return remember { store.data[KEY_NAV_RESULT] as Result? }
}

/**
* Provide (create or restore) viewModel instance bound to navigation entry
*
* @param key provides unique key to create viewModel
* @param provider default viewModel instance provider
*/

@Composable
@Suppress("UNCHECKED_CAST", "CastToNullableType", "UnusedReceiverParameter")
inline fun <reified Model : TiamatViewModel> NavDestinationScope<*>.rememberViewModel(
noinline provider: () -> Model
): Model = rememberViewModel(className<Model>(), provider)

/**
* Recommended to use `rememberViewModel(key, provider)` instead
*
* Provide (create or restore) viewModel instance bound to navigation entry
*
* @param key provides unique key part
* @param provider default viewModel instance provider
*/
@Composable
@Suppress("UNCHECKED_CAST", "UnusedReceiverParameter")
fun <Model : TiamatViewModel> NavDestinationScope<*>.rememberViewModel(
key: String? = null,
key: String,
provider: () -> Model
): Model {
val store = LocalDataStore.current ?: error("Store not bound")
val compositionHash = currentCompositeKeyHash
return remember {
val storeKey = "Model#" + (key ?: compositionHash.toString(16))
val storeKey = "Model#$key"
store.data.getOrPut(storeKey, provider) as Model
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,21 @@ class NavController internal constructor(
val key: Any?,
val storageMode: StorageMode,
val startDestination: NavDestination<*>?,
internal val dataStorage: DataStorage,
private val savedState: Map<String, Any?>?,
private val destinations: Array<NavDestination<*>>
) {
companion object {

private var nextUUID = 0L

const val KEY_CURRENT = "current"
const val KEY_BACKSTACK = "backStack"
const val KEY_UUID = "uuid"
}

// needs to be restored asap not to rent extra uuid's
private val uuid: Long = (savedState?.get(KEY_UUID) as? Long) ?: nextUUID++

/**
* provides current active NavDestination as State object
*/
Expand All @@ -43,6 +49,8 @@ class NavController internal constructor(

internal var currentNavEntry by mutableStateOf<NavEntry?>(null)
private set
internal var dataStorage: DataStorage = DataStorage()
private set
internal var isForwardTransition = true
private set
internal var isInitialTransition = true
Expand All @@ -51,6 +59,8 @@ class NavController internal constructor(
private set

init {
// ensure next uuid will be unique after restoring state of this one
nextUUID = maxOf(nextUUID, uuid + 1)
val namesSet = mutableSetOf<String>()
val duplicates = arrayListOf<String>()
destinations.onEach {
Expand Down Expand Up @@ -202,25 +212,33 @@ class NavController internal constructor(
}
}

internal fun reset() {
internal fun toSavedState() = currentNavEntry?.let { entry ->
mapOf(
KEY_UUID to uuid,
KEY_CURRENT to entry.apply { saveState() }.toSavedState(storageMode),
KEY_BACKSTACK to backStack.map { it.toSavedState(storageMode) }
)
}

internal fun restoreState(parentDataStorage: DataStorage) {
val storageKey = "DataStore#$uuid"
dataStorage = parentDataStorage.data.getOrPut(storageKey) { DataStorage() } as DataStorage
if (storageMode == StorageMode.DataStore.ResetOnDataLoss && dataStorage.data.isEmpty()) reset()
else restoreFromSavedState()
}

private fun reset() {
backStack.clear()
setCurrentNavEntry(null, false)
if (startDestination != null) navigate(startDestination)
}

internal fun restoreFromSavedState() {
private fun restoreFromSavedState() {
savedState?.let(::restoreFromSavedState)
if (currentNavEntry == null && startDestination != null)
navigate(startDestination)
}

internal fun toSavedState() = currentNavEntry?.let {
mapOf(
KEY_CURRENT to currentNavEntry?.apply { saveState() }?.toSavedState(storageMode),
KEY_BACKSTACK to backStack.map { it.toSavedState(storageMode) }
)
}

@Suppress("UNCHECKED_CAST")
private fun restoreFromSavedState(savedState: Map<String, Any?>) {
runCatching {
Expand Down
23 changes: 14 additions & 9 deletions library/src/commonMain/kotlin/com/composegears/tiamat/NavEntry.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@ import androidx.compose.runtime.saveable.SaveableStateRegistry
*
* Hold nav entry information and allow to save/restore state base on storage mode
*/
internal class NavEntry(
internal class NavEntry private constructor(
val uuid: Long,
val destination: NavDestination<*>,
var navArgs: Any? = null,
var navResult: Any? = null,
var entryStorage: DataStorage? = null,
var savedState: Map<String, List<Any?>>? = null,
var savedStateRegistry: SaveableStateRegistry? = null,
) {
companion object {

Expand All @@ -30,20 +27,28 @@ internal class NavEntry(
storageMode: StorageMode,
destinations: Array<NavDestination<*>>,
) = NavEntry(
uuid = this[KEY_UUID] as Long,
destination = (this[KEY_NAME] as String).let { name -> destinations.first { it.name == name } },
savedState = this[KEY_SAVED_STATE] as? Map<String, List<Any?>>?,
).also {
it.uuid = this[KEY_UUID] as Long
// ensure next uuid will be unique after restoring state of this one
nextUUID = maxOf(nextUUID, it.uuid + 1)
it.savedState = this[KEY_SAVED_STATE] as? Map<String, List<Any?>>?
if (storageMode == StorageMode.Savable) {
it.navArgs = this[KEY_NAV_ARGS]
it.navResult = this[KEY_NAV_RESULT]
}
}
}

internal var uuid: Long = nextUUID++
private set
var navResult: Any? = null
var entryStorage: DataStorage? = null
var savedState: Map<String, List<Any?>>? = null
var savedStateRegistry: SaveableStateRegistry? = null

constructor(
destination: NavDestination<*>,
navArgs: Any? = null,
) : this(nextUUID++, destination, navArgs)

internal fun saveState() {
savedState = savedStateRegistry?.performSave()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.composegears.tiamat

/**
* We can not call T::class in @Composable functions,
*
* workaround is to call it outside of @Composable via regular inline fun
*/
inline fun <reified T : Any> className(): String = T::class.qualifiedName!!

0 comments on commit 8bdbca8

Please sign in to comment.