I was looking at <@UJBPFB3SN>’s <Type safe, multi-...

# compose

[julioromano]

I was looking at @Ian Lake’s

Type safe, multi-module best practices with Navigation Compose▾

video and was trying to apply the pattern suggested in it to an app of mine.

I’ve run in a situation that seems to break the pattern proposed in the video as soon as I had to do “conditional navigation”. 🧵

[julioromano]

Example of conditional navigation:
- user taps logout button.
- logout business logic runs.
- if logout logic fails then navigate to an alert dialog.
- if logout logic succeeds then empty the back stack and navigate to the welcome screen.

The problem is that business logic runs in the ViewModel but navigation actions must be invoked on the NavController object which is inside the composition in the Activity.

Since the lifecycle of the Activity is shorter than that of the ViewModel we can’t pass references to functions such as NavController.popUpToWelcomeScreen() from the Activity to the ViewModel (besides the potential of memory leaks, we’ll loose the reference to the function any time the activity is recreated).

How to properly encapsulate conditional navigation using the pattern proposed in the video?

[Pablichjenkov]

You set an ObservableState/ReactiveState in the ViewModel that your composable function subscribe to observe changes and react accordingly.
Sort of:

@Composable
fun LogoutScreen(accountViewModel: AccountViewModel) {
  val loginStatus by accountViewModel.loginStatus.collectAsState()
  when(loginStatus) {
    LogIn -> { paintLogoutUI() }
    LogOut -> { navController.navigate("WelcomePage") }
  }
}

You don’t pass a reference from anywhere to the ViewModel, so the ViewModel invoke/callback later on, instead invert the flow. You request execution to the ViewModel and listen for the result of the execution. The ViewModel doesn;t know about its client, doesn’t call functions in its clients

[Ian Lake]

See https://manuelvivo.dev/viewmodel-events-antipatterns

[Colton Idle]

I've been in this situation before, and basically the solution here is that your logout failed and logout succeeded code flows should not trigger a logout failed event or a logout succeeded event. instead they should update some state that you observe in your composable. and then your composable reacts to that state. pretty much what the people above me said.

checkout these docs too: https://developer.android.com/topic/architecture/ui-layer/events#handle-viewmodel-events

[julioromano]

The proposed pattern works, thanks! It is indeed a valid solution but, from a purely architectural PoV, wouldn’t it be all leaner if the NavHost would allow its state to be hoisted so that we could directly manipulate it?

After all isn’t Jetpack Compose about manipulating hoisted state to trigger updates to the UI instead of having to manually change a `View`’s internal state by calling its setters?
Every time I do a navController.navigate() I feel like I’m doing something like textView.setText() .

[Ian Lake]

NavController is already the hoisted state of the NavHost. navigate is just how you update the state of the NavController, just like how you call open or close on a DrawerState

[Ian Lake]

A NavController is indeed tied to the composition lifecycle and can't be hoisted into a ViewModel, that is true though. There are quite a few composition state holders that can't be hoisted out of composition safety though - ScrollState looks like it can be hoisted, but any attempt to change the state from a ViewModel actually just crashes..

[Pablichjenkov]

Any rule for that Ian? - for determining if a State has to live within the composition lifecycle. I bet is usually state containing layout/drawing information about the composable itself?
It is very tempting to have a reference of the navController out of the composable tree.

[Ian Lake]

In this case, you'll leak the whole activity and every Navigator (which also has hard references to other things in composition), so that should show up right away when you use LeakCanary (and won't work either)

[Pablichjenkov]

I see, yeah that’s another point to consider, leakage, ok

[julioromano]

There are quite a few composition state holders that can’t be hoisted out of composition safety though - ScrollState looks like it can be hoisted, but any attempt to change the state from a ViewModel actually just crashes..

This opens the pandora’s box of architectural opinions: Should a given composable’s state be manageable by the ViewModel/BusinessLogic or should it be exclusive to the UI layer?

I don’t think there’s a definitive answer for this. Take ScrollState for instance, in most cases most people would be okay with it being exclusive to the UI layer, so that the business logic can be agnostic about it and the UI will transparently take care of restoring the scroll position of a list.
But things change as soon as the business requirements say that the list must be programmatically scrolled to a certain position whenever a certain business operation has been completed. In this case we would like for the ScrollState to be hoisted to our ViewModel/BusinessLogic to better control it.

So maybe it boils down to a matter of balancing “how complicated is it to make a certain state work out of the composition” with “how useful is it to actually be able to hoist a certain state out of the composition”.

IMHO (just my 2 cents that are worth much less than that 🙂 ) for ScrollState the usefulness would be minimal, but for NavHost I’d say quite the opposite.

[Colton Idle]

woah. thanks for the info Ian

NavController is already the hoisted state of the NavHost. navigate is just how you update the state of the NavController, just like how you call open or close on a DrawerState

🤯

@Manuel Vivo i guess this also goes hand in hand with how we talked about the fact that you can't actually hoist the state of a bottom sheet into a ViewModel

[Manuel Vivo]

We added more info about that topic @Colton Idle 🙂 you can hoist it there but you gotta be careful. We added that caveat to the docs as well: https://developer.android.com/jetpack/compose/state-hoisting#caveat

[julioromano]

It doesn’t seem that NavHost falls in that caveat’s topic though, navController.navigate() is not suspending.

[Pablichjenkov]

An open Pandora box 😁 funny, but I prefer the Android freedom over other platforms that force you to do stuff one unique way. The Android team just "recommend" stuff 🙂