Tlaster / PreCompose

Compose Multiplatform Navigation && State Management
https://tlaster.github.io/PreCompose/
MIT License
847 stars 49 forks source link

ViewModel cached for same compose funtions #307

Open ellykits opened 5 months ago

ellykits commented 5 months ago

Describe the bug

I'm using Precompose in an Android application, I however noticed each time I navigate to the same Scene that uses the same ViewModel, the same ViewModel instance is returned.

To Reproduce For context see a similar issue reported on voyager https://github.com/adrielcafe/voyager/issues/7

Expected behavior A new ViewModel should be constructed with each transition to a scene.

Minimal reproducible example Very similar to https://github.com/adrielcafe/voyager/issues/7

Tlaster commented 5 months ago

Can you provide more example of this bug? since I'm not familiar with voyager and cannot reproduce this issue.

markosopcic commented 5 months ago

Hey! Not connected to op, but i have the same issue and I've assembled a simple example for this.

Code example ```kotlin import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.material.Card import androidx.compose.material.Text import androidx.compose.material.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp import androidx.compose.ui.window.Dialog import androidx.compose.ui.window.Window import androidx.compose.ui.window.application import kotlinx.coroutines.flow.MutableStateFlow import moe.tlaster.precompose.PreComposeApp import moe.tlaster.precompose.koin.koinViewModel import moe.tlaster.precompose.viewmodel.ViewModel import org.koin.core.context.startKoin import org.koin.dsl.module fun main() = application { startKoin { modules(module) } Window(onCloseRequest = ::exitApplication, title = "Bug") { PreComposeApp { var isDialogShown by remember { mutableStateOf(false) } Box(contentAlignment = Alignment.Center, modifier = Modifier.fillMaxSize()) { TextButton(onClick = { isDialogShown = true }) { Text("Click Me") } } if (isDialogShown) { RandomDialog { isDialogShown = false } } } } } @Composable fun RandomDialog(onDismiss: () -> Unit) { val vm = koinViewModel() val state by vm.initialized.collectAsState() Dialog(onDismissRequest = onDismiss) { Card { Column(modifier = Modifier.padding(16.dp)) { TextButton(onClick = { vm.initialized.value = !vm.initialized.value }) { Text("Initialized: $state") } Spacer(modifier = Modifier.height(12.dp)) TextButton(onClick = onDismiss) { Text("Dismiss") } } } } } val module = module { factory { BasicViewModel() } } class BasicViewModel : ViewModel() { var initialized = MutableStateFlow(false) } ```

Repro steps:

  1. Start app
  2. Click "Click Me" in the center of the app to open the dialog
  3. Click "Initialized: false" to toggle the VM state and the state of the button to "Initialized: true"
  4. Click "Dismiss to dismiss the dialog
  5. Click "Click Me" again to again open the dialog.
  6. The state of the button is "Initialized: true", meaning the same VM as the first time was returned.

Expected behaviour: Every time the dialog is opened, the koinViewModel function should return a new view model instance.

Let me know if I can help any other way.

Tlaster commented 5 months ago

The reason koinViewModel returns the same instance is the same as #294, koinViewModel is seeking for viewModel from a StateHolder, which in this case is the root StatsHolder from PreComposeApp, so it does return the same viewModel every time, this is the current behaviour.

markosopcic commented 5 months ago

Ah I see. In android it seems like a composable function in itself is a state holder but of course I'm not sure how it works under the hood. Here we just need to work around this by clearing/resetting the view model when the composable dismisses. Something like this will make this easier so i don't have to rewrite it every time for each composable that uses a VM.

@Composable
inline fun <reified VM : ViewModel> ComposableWithVM(vm: VM = koinViewModel<VM>(), content: @Composable (VM) -> Unit) {
    DisposableEffect(Unit) {
        onDispose {
            vm.clear()
        }
    }

    content(vm)
}

Thank you for the answer!