Hi! Found a bug with Compose Navigation version `2...

# compose

[Rafael Costa]

Hi!
Found a bug with Compose Navigation version 2.8.0-alpha06. I haven't found the root cause and I don't have a lot of time, but I have a case where the SavedStateHandle is empty where a nav arg was sent.
If I just go back to 2.8.0-alpha05, everything works well again.
I found that the bundle on NavBackStackEntry does have all arguments as expected, so it's something related specifically to SavedStateHandle.
Would report an issue, but tbh I am really short on time 😬

[Rafael Costa]

@Ian Lake
hope just the info that something is definitely wrong can help. I can try to explore deeper if I get some time in the days to come, but no promises 😞

[Ian Lake]

I can't see how that's possible if you're using the default ViewModel factory (e.g., just a regular viewModel() call), so I'd check that first to rule out any issue on the Navigation side. Otherwise, worth tracing whatever DI system (if any) you're using to provide your ViewModels

[Rafael Costa]

I can check it, but there’s definitely a breaking change with this version since with alpha05 everything is working.
It’s true that I am probably using an “old way” to get my ViewModels with manual DI and my own viewModel function, but still something was changed here.
Is there any intentional breaking change in this version?

[Rafael Costa]

Btw, this is my viewModel function:

@Composable
inline fun <reified VM : ViewModel> viewModel(
    viewModelStoreOwner: ViewModelStoreOwner = checkNotNull(LocalViewModelStoreOwner.current) {
        "No ViewModelStoreOwner was provided via LocalViewModelStoreOwner"
    },
    savedStateRegistryOwner: SavedStateRegistryOwner = LocalSavedStateRegistryOwner.current
): VM {
    return androidx.lifecycle.viewmodel.compose.viewModel(
        viewModelStoreOwner = viewModelStoreOwner,
        factory = ViewModelFactory(
            owner = savedStateRegistryOwner,
            defaultArgs = (savedStateRegistryOwner as? NavBackStackEntry)?.arguments,
             //remove this line if you're not using Dependency injection
            dependencyContainer = [ACCESS YOUR DEPENDENCIES GRAPH HERE SOMEHOW],
        )
    )
}

[Rafael Costa]

ViewModelFactory is a AbstractSavedStateViewModelFactory to which I pass those defaultArgs

[Rafael Costa]

Found it!
It's actually a mistake on my part, although it is true that there was some behaviour change here. Maybe it was a bug that was fixed and my code was counting on it? 😛

Basically, I am getting the same viewModel outside the screen by calling that same function shown above. It seems like with version alpha05 and older, I was successfully getting the same VM I am getting on the screen, by just passing the viewModelStoreOwner (which was the NavBackStackEntry).
With version alpha06 and above, I need to use that NavBackStackEntry on both the viewModelStoreOwner and the SavedStateRegistryOwner. This does sort of make sense to me, I just wonder how it worked before. I'm guessing by providing the ViewModelStoreOwner, even if the SavedStateRegistryOwner was not correct, it wouldn't matter since the same VM would be returned.

[Ian Lake]

Sounds like you might be overriding the CreationExtras version of create, which pulls the arguments from the ViewModelStoreOwner who implements HasDefaultViewModelProviderFactory (which Activity, Fragment, NavBackStackEntry) etc. all implement). The parameters you pass to the constructor are really only for the compat story if you are using a new version of Lifecycle with an old version of older libraries

[Ian Lake]

I wonder if there's been a change Lifecycle 2.8 (which the latest Navigation would pull in transitively via Compose) that change the priority order

[Ian Lake]

Tbh, you haven't needed AbstractSavedStateViewModelFactory since Lifecycle 2.5. The CreationExtras approach has been enough since then

[Rafael Costa]

Yap, I’ve known about CreationExtras for a while. But the current approach has served me well, so I’ve been unconsciously delaying the changes to start using it 😅