Is it considered a very bad practice to pass down ...

# compose

[Vladimir]

Is it considered a very bad practice to pass down a NavController like so:

fun NavGraphBuilder.homeScreenGraph(navController: NavController) {
    composable<Home> {
        HomeScreen(navController)
    }
}


@Serializable
object Home

[Stylianos Gakis]

If you use it to navigate, probably yes since wherever you're passing it to might not even have a reference to the destination you want to navigate to if those two things are in separate modules.
You can always make it a lambda instead and write where you're gonna be navigating to right here instead.

If you're using it just to call popBackstack/navigateUp and other things like grabbing an entry from the backstack then no that's probably perfectly fine.

[Vladimir]

What about changing the Home object into a data class and passing the navcontroller through it?

[Vladimir]

would that be considered more acceptable?

[Stylianos Gakis]

Actually it'd be nice to know what you're trying to achieve in the first place.

Having a reference to the NavController inside the Home class itself does not sound like a good idea, no.

[Vladimir]

I am following the best practices for jetpack navigation video on youtube that was made by google (see the picture attached).

Since the Home navigation needs to be able to be navigated from to other screens, I am trying to figure out the best way to pass down the navcontroller while following the example in the screenshot

[Stylianos Gakis]

Your composable can take in a onNavigateToHome: () -> Unit so they don't need to know about Home whatsoever.
Then when you call the composable, you pass onNavigateToHome = { navController.navigate(Home) }

[Stylianos Gakis]

If you are not defining a composable directly but an extension on NavGraphBuilder, as the video shows, the same idea applies. Same lambda and you provide it the same way.

[Vladimir]

I see, thank you. One more quick question - in the provided screenshot example, they don't pass in the view model but rather instantiate it in place. I thought this was an antipattern as it does not favor loose coupling? What are your thoughts of them doing them in the example?

[Stylianos Gakis]

Where else would you instantiate it?
You want it to be done exactly there since you want the VM to be scoped to the destination itself so that it also gets destroyed after you go back and leave that destination.
If you "pass it in" from somewhere above there, the VMs lifecycle won't be scoped to your destination.
Doing it there uses the right composition local to make that happen.
What the screenshot shows is exactly what you wanna be doing.

[Vladimir]

Thank you

[Vladimir]

@Stylianos Gakis Your previous message about the VM lifecycle made me carefully reconsider some of the choices I had made and raised some doubts.

I have a session manager VM responsible for verifying/ loging in the user that I pass into the wrapping MyApp composable using hiltViewModel

So my MainActivity looks like this:

@AndroidEntryPoint
class MainActivity : AppCompatActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            MyApp()
        }
}

And then my MyApp composable looks like this:

@Composable
fun MyApp(sessionViewModel: SessionViewModel = hiltViewModel()) {
    //todo theme wrapper is likely to go here
    val navController = rememberNavController()
   MyNavHost(navController, sessionViewModel)

    if(sessionViewModel.sessionStatus.value) {
        navController.navigate(route = Home)
    }
    else {
        navController.navigate(route = Login)
    }

    LaunchedEffect(sessionViewModel.sessionStatus.value) {
        if (sessionViewModel.sessionStatus.value) {
            navController.navigate(Home) {
                popUpTo(Login) { inclusive = true }
            }
        }
    }

}

Having carefully read what you wrote I'd like to kindly ask you the following:

1. Do you agree with this SessionViewModel injection?
2. Is it correct to assume that the lifecycle of the MyApp composable will persist throughout the lifespan of the app, because it is the main wrapping composable? So even if within it I navigate to other screens it will still be sensitive to state changes, such as if I ever choose to invalidate the session?

[Stylianos Gakis]

Hey yes, that is a very good question!
So the way that hiltViewModel knows to scope the VM to the destination you are currently in is by grabbing the LocalViewModelStoreOwner.current and scoping it to the ViewModelStoreOwner. You can see this in the source code of it right here https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:hilt/hil[…]t/navigation/compose/HiltViewModel.kt;l=46-47?q=hiltViewModel
So the ViewModelStoreOwner is set for each destination as a composition local when you write composable<Home> {...}.
But when you are outside of any navigation destination, your LocalViewModelStoreOwner.current will point to your Activity. Which is itself also a ViewModelStoreOwner as seen here https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:activity[…]roidx/activity/ComponentActivity.kt;l=115?q=ComponentActivity

So that VM will be scoped to the entire Activity, and if you are in a single activity app which you most likely should be in, then yes this will persist for the entire lifecycle of your app.
So what you are doing looks good to me!

[Stylianos Gakis]

This is not part of your question, but the one thing which is concerning is this part

if(sessionViewModel.sessionStatus.value) {
        navController.navigate(route = Home)
    }
    else {
        navController.navigate(route = Login)
    }

where you are doing a side effect in composition instead of inside a side-effect.
Right below it you do it right inside a side effect. You will want to avoid doing the side-effect just in composition like that, since this may be called even tens of times in one second if your composable just happens to be recomposing.

[Vladimir]

@Stylianos Gakis Great response and lots of material, thank you very much!