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 not cleared when screen is disposed #210

Closed
slickorange opened this issue Sep 20, 2023 · 10 comments · Fixed by #242
Closed

Viewmodel not cleared when screen is disposed #210

slickorange opened this issue Sep 20, 2023 · 10 comments · Fixed by #242
Labels
bug Something isn't working

Comments

@slickorange
Copy link

I am using AndroidScreen with HiltViewModel

When the screen is disposed, the viewmodel is not cleared.

object InviteCodesScreen : AndroidScreen() {
    @Composable
    override fun Content() {
        val screenModel = getViewModel<InviteCodesViewModel>()
        InviteCodesScreenUI(viewModel = screenModel)
        LifecycleEffect(
            onStarted = {
                screenModel.load()
            },
            onDisposed = {
                Timber.i("The screen is disposed...")
            })
    }
}
@HiltViewModel
class InviteCodesViewModel @Inject constructor(
    application: Application,
    val accessManager: AccessManager
) : BaseViewModel(application) {

    var uiState by mutableStateOf(InviteCodesUiState())

    fun load() {
        uiState = uiState.copy(accessDefinitions = accessManager.shareableDefinitions ?: emptyList())
    }

    override fun onCleared() {
        Timber.i("Viewmodel cleared")
    }

Another issue, which I am not sure is related, is that screens of the same type share viewmodels. Eg. I have a LessonScreen which displays a lesson. When I go from "lesson A" to "lesson B" by pushing a new LessonScreen the screen for LessonB does not get it's own instance of the viewmodel (this was the behaviour when I used RC 5, but not what I see with 7)

I am pretty stuck and would appreciate any help

@KoTius
Copy link

KoTius commented Sep 20, 2023

  1. Probably because you use singleton for your screen.
  2. Because you don't provide a tag when you get the viewModel.

@programadorthi
Copy link
Collaborator

I found the issue in the pull request #162 from @aftabahmadTW that removed the LifecycleOwner responsible to manager the ViewModel lifecycle. Without that the ViewModel lifecycle will be scoped to current Activity and keeping alive until the Activity is destroyed.
CC @adrielcafe @DevSrSouza

@programadorthi programadorthi added the bug Something isn't working label Sep 20, 2023
@slickorange
Copy link
Author

  1. Probably because you use singleton for your screen.

    1. Because you don't provide a tag when you get the viewModel.

As I mentioned, this used to work and only started happening once I updated to the latest version. (Without making any changes to my viewmodels or navigation.

Also not sure what you mean by providing a tag - I am just using the method showed in the documentation..

@programadorthi
Copy link
Collaborator

@slickorange try rollback to RC06. Pull request #162 is on the RC07. We will try fix that 👍

@slickorange
Copy link
Author

I am having trouble with RC06 and the latest version of Jetpack compose.

@osrl
Copy link
Contributor

osrl commented Sep 27, 2023

I'm on RC06 and it's not working

@slickorange
Copy link
Author

Any updates on this?

@slickorange
Copy link
Author

slickorange commented Oct 23, 2023

I am still seeing this issue with the latest version (1.0.0-rc08)

@DevSrSouza
Copy link
Collaborator

The main issue here is that we change the behavior of AndroidScreenLifecycle, removing it from the AndroidScreen and injecting it by default for all screens.
Your implementation of hilt viewModel extension was based on the fact that the Screen should be a AndroidScreenLifecycle. I figure this out late yeasterday.

The PR #212 from @programadorthi fix that issue by not casting Screen.lifeycleOwner to AndroidScreenLifecycle and instead using LocalLifecycleOwner, we should try reopen it and merge it to next release.

@DevSrSouza
Copy link
Collaborator

Fixed on 1.0.0-rc09

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants