Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ViewModel cached for same compose funtions #307

Open
ellykits opened this issue Apr 15, 2024 · 4 comments
Open

ViewModel cached for same compose funtions #307

ellykits opened this issue Apr 15, 2024 · 4 comments

Comments

@ellykits
Copy link

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 adrielcafe/voyager#7

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

Minimal reproducible example
Very similar to adrielcafe/voyager#7

@Tlaster
Copy link
Owner

Tlaster commented Apr 17, 2024

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

@markosopcic
Copy link

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

Code example
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<BasicViewModel>()
    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
Copy link
Owner

Tlaster commented Apr 20, 2024

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
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants