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

Hilt viewmodel + replace + transition = crash #17

Closed
Tolriq opened this issue Oct 12, 2021 · 3 comments
Closed

Hilt viewmodel + replace + transition = crash #17

Tolriq opened this issue Oct 12, 2021 · 3 comments

Comments

@Tolriq
Copy link

Tolriq commented Oct 12, 2021

So this is a tricky one.

Navigator(Screen1()) { navigator ->
                        SlideTransition(navigator)
                    }

class Screen1() : AndroidScreen {
    @Composable
    override fun Content() {
            val viewModel = getViewModel<Screen1ViewModel>()
            ....
            LocalNavigator.currentOrThrow.replace(Screen2())
    }
}

Will lead to a crash

    java.lang.IllegalArgumentException: SavedStateProvider with the given key is already registered
        at androidx.savedstate.SavedStateRegistry.registerSavedStateProvider(SavedStateRegistry.java:111)
        at androidx.lifecycle.SavedStateHandleController.attachToLifecycle(SavedStateHandleController.java:50)
        at androidx.lifecycle.SavedStateHandleController.create(SavedStateHandleController.java:70)
        at androidx.lifecycle.AbstractSavedStateViewModelFactory.create(AbstractSavedStateViewModelFactory.java:67)
        at androidx.lifecycle.AbstractSavedStateViewModelFactory.create(AbstractSavedStateViewModelFactory.java:84)
        at dagger.hilt.android.internal.lifecycle.HiltViewModelFactory.create(HiltViewModelFactory.java:109)

From a quick look the viewmodel is properly cleared, but it seems Voyager tries to re access the SaveStateProvider later with the same key (obviously as it's the same composable).

There's no issue without enabling transitions, so this looks like some wrong order of calls, whatever access the savestate during the transition should do it before the full removal of the composable to avoid trying to reuse the key.
There's also no problem when just pushing a new screen. This works without using hitviewmodel.

@Tolriq
Copy link
Author

Tolriq commented Oct 14, 2021

After looking more into this, this is a more general issue about auto dispose and transitions.

So this probably becomes more a feature request as all the auto dispose / disposal stuff is internal it's not really possible to do it manually at the end of the transition.

Tl;DR is: Auto dispose dispose things too early when using transitions. (And not using auto dispose is not really an option for large apps)

Leading to a few side effects like :

  • Full recreation of the view/screen models during the transition (not memory/cpu friendly at that timing). (Leading to this crash that is specific to hilt and viewmodel)
  • No disposing of the recreated instances

@programadorthi
Copy link
Collaborator

Fixed just putting a missed remember on ViewModel creation here #18. Checkout solution

@adrielcafe
Copy link
Owner

Fixed on 1.0.0-beta13, feel free to reopen if the issue persists.

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