is there a consensus on what is “best practice” wh...

# compose

[james]

is there a consensus on what is “best practice” when using the Navigation Compose library? if I look around at many examples in open source projects I can see 3 methods being used:
1. Passing lambdas from the top level of the Composable hierarchy (where the NavController is), all the way down to where it’s required, eg:
a. above where a NavHost is declared we see something like val navigateToDetailScreen: (Int) -> Unit declared
b. navigateToDetailScreen is passed into the nav graph builder and passed into every composable() route that might need to call it
c. navigateToDetailScreen is then passed through as many Composables as required until it is passed into to the Composable that will generate the action from an onClick event
2. Passing the instance of NavController from the same top level (where the NavHost is declared) down to each screen and into the ViewModel which will then keep a reference to it
3. Wrapping NavController in a custom navigator class and injecting that custom class into ViewModels where it’s required
if I think about how best to build something flexible, testable, and not too “polluting” of my codebase, #3 seems like the best option, but when I look at examples of what others have built that method seems like the least common, so I find I am second guessing myself and wondering if there’s a problem with that option which I’m not seeing

[Ian Lake]

You cannot inject any wrapper around a NavController into a ViewModel as that will leak your activity after a configuration change (the NavController only exists at the UI layer for exactly that reason) so #3 is not even an option when using Navigation Compose

#2 is, of course, a terrible idea for testability, something we explicitly cover in the docs (none of your screen should ever have a dependency on a particular Navigation framework): https://developer.android.com/jetpack/compose/navigation#testing

[Ian Lake]

We had talked about that idea of layered structure and keeping your Navigation code out of your individual screens previously: https://kotlinlang.slack.com/archives/CJLTWPH7S/p1645595702068129?thread_ts=1645552485.247699&cid=CJLTWPH7S

[Colton Idle]

I do option 1 and it just works. nice to not have a dependency on the nav framework. Shipped 3 fully compose apps so far. seems legit.

[james]

alright cool, thank you both. we’re using option 1 currently so it sounds like we should be sticking with that

[Colton Idle]

yeah. we try to stick with the mantra of state down, events up.

[Colton Idle]

which in practice means... state (and event lambdas) passed down and events bubble back up to the thing that cares.

[james]

yeah, totally clear on that part.. it’s just a discussion point that has come up amongst some few engineers I work with a bunch of times, with the two contentious points (of option 1) being:
• some screens end up having many navigateToX parameters passed in
• traditionally, we would simply tell the ViewModel that a click event happened and the ViewModel would own the logic which decides what that means. eg:
◦ we clicked ButtonA, but we might want to either navigate to ScreenX or ScreenY, depending on some logic
◦ the ViewModel will also send what happened here back to our analytics engine
the second point is the more pressing one.. when the Composable owns the navigation event, you can still trigger this stuff from the ViewModel (analytics and deciding which screen to nav to, etc) but it becomes quite verbose

[Ian Lake]

FWIW, there's also a really good blog post about one off events and ViewModels: https://medium.com/androiddevelopers/viewmodel-one-off-event-antipatterns-16a1da869b95

[dewildte]

I design my screen Composables to be dependent on a state interface and maybe a few flag parameters.
No dependcies that can't be easily made in a preview function.

Then I wrap my screen in a *Controller Composable that knows all about l Navigation, ViewModels, WindowSizeClass, and other infrastructure.

This pattern is repeated down my Compose Tree

[dewildte]

In the future if I wanted to put my screen and component definitions into a module it would be pretty easy. Also previews are really easy to maintain this way.

[dewildte]

The events a particular screen produces are represented by a sealed interface. And passed up into a single event handler lambda. Almost like MVI.
My Controller Composables then handle the events, either calling navigations on the navhostcontroller or mapping the events into command classes which in turn are piped up into a ViewModel for processing.