Hey! My team has been diving a little more deeply ...

# compose

[fal]

Hey! My team has been diving a little more deeply into Nav3 but there's a few things that have been bothering us around the patterns. First. we REALLY dislike the pattern in NowInAndroid, where each feature knows about navigation, and depends on other features api modules to know about NavKeys to navigate. We think this is a dependency mess and hard to follow and reason with. We've been exploring just having a centralized navigation module that knows about all features, and making it so that the feature itself knows nothing about navigation, it just bubbles up lambdas that the navigation then knows what to do.

We're fairly happy with the approach, but then comes a little snag: when we need a result from another screen. There are recipes about it here and here. The only thing we don't like is that using these approaches kind of means we need to wire the VM + Screen at the navigation level, while ideally we wanted that wiring to be done at the feature level. Ideally, navigation would only know about "routes", not how to wire VM + Screen -- because this means we need to make the ViewModel public, and again, we didn't really want it.

We only currently see two ways out of this: making the features aware of navigation (which we really didn't want), or having the wiring on the navigation module which we also didn't want. Did anyone come across this? Any suggestion? Thanks!

[Hristijan]

Ideally features should be independent and navigation events should be dispatched to a centralized event handler that only lives in your activity, your features should only have a callback mechanism that dispatch further to your view model in order to process results from other screens nothing more than that

Your navigator dispatcher should be injected inside the view model and your screens should be only sending events down to it, they should not be in charge of navigating, same with passing results and navigating back

That's my opinion btw

[fal]

Thanks for the reply, what I said in my post basically agrees with what you said, just that we go even further. We do not have any concept at all of navigation in the feature including any "navigator dispatcher". That is still a dependency on navigation. If your ViewModel does "NavigateToX" through a dispatcher, even if it's just an event, it still implicitly knows about navigation.

What we were doing is just bubble up lambdas from the screen / vm. It just reports "this happened" (e.g. authentication finished), it does not do "navigateToHome". It's up to the navigation module to decide to translate "authentication finished" to "navigate to home".

Our issue has more to do with navigation results as described here, and how that fits into our pattern. The pattern requires ResultEffect to be registered at the same level as the viewModel instantiation, which presents the issue I described above.

[sindrenm]

[…] where each feature knows about navigation, and depends on other features api modules to know about NavKeys to navigate

I'm quite sure the reason for this, and not some :core:navigation:nav-keys module that holds all the nav keys, is to avoid unnecessarily building modules that don't need to be rebuilt just because one nav key was added, changed or removed.

[sindrenm]

Although I suppose that doesn't really matter in your approach, as I'm reading it now.

[fal]

Well, but we just have all the navkeys in the navigation module... If one changes, it just rebuilds the navigation module, but since the features themselves do not depend on the navigation module (it's the other way around), it doesn't affect the feature modules at all

[fal]

Yep that's it

[Ian Lake]

How big is your team? The whole reason we specifically steer folks away from the one centralized module is that it does not scale. In a world where they are completely separate team owning feature modules, having the navigation logic in that module and having that module own what dependencies it requires is a local decision, not a global one

[fal]

Team is around ~40 people, and the project is definitely not small, but I'm sure there are bigger teams out there. I do understand the motivation on trying to decentralize navigation and scalability, but it becomes difficult to try and reason any navigation flow around our app with the NIA pattern. Further (and this is specific for our project), our feature :api / :impl modules already focus on the data/domain layers, while :ui is, well, more for ui related stuff. I can't give you a good argument there, but doesn't feel great for a feature to just depend on another because it needs to navigate to it, and otherwise doesn't have any other relationship.

I'll concede though that there is definitely a fair amount of subjectiveness on what I just said, so it's fair to say that the NIA pattern is simply the recommended one

[Ian Lake]

What are you "trying to reason about" that you find difficult? What adds a particular screen to the back stack? That's a Find Usages of the key. What screens a given module navigates to? That is centralized in each module in their EntryProviderScope extension methods (since screens themselves should always be independent of navigation).

The names :api and :impl are supposed to mean something though: what you publicly expose specifically to other modules (your API) versus your implementation details. Are you not using those names to mean those things?

[fal]

I'd say reasoning about the app navigation as whole instead of isolated blocks, also if we have navigation completely isolated, it means we have the flexibility to replace the navigation system easily (yeah, we shouldn't, but we're at nav3 now -- will there be a 4? 😂). If I'm quite honest with you, our current navigation system is not like what I mentioned yet, it's just that as we're completely redoing the navigation system, and this approach I mentioned actually seemed pretty good to us. And also, conceptually, I still think it makes perfect sense to completely separate feature from navigation. You already do so at the screen composable level, and it makes sense to go even further and completely take out navigation from the feature. Navigation as a concept to me always seemed to be global to the application, not local to the feature, and this seemed like a great step in that direction.

You can disregard the :api :impl arguments, as obviously we could easily adjust to it (and yes, api and impl does mean that to us, it's just that implicitly we have been more using it to mean the api/impl of data/domain layers, and cornering ui related stuff by itself, not that we couldn't adjust.

[Ian Lake]

I think you'll regret your choices almost immediately, to be honest. Centralizing always feels like a good idea, right up until it grows bigger than any single person can keep in their mind, then you seek to split it into smaller pieces you can keep in your mind all at once.

It turns out those smaller pieces that you can continue to reliably reason about are about ~feature/module level 🙃

That just nicely ends up aligning with team boundaries too where each team is then able to own their flows (swapping from 3 screens to 4 when a new compliance screen comes up in the onboarding flow in a specific country; keep in mind that not every key needs to be publically accessible, it could just be the entry points to the feature)

[Ryan Payne]

@fal how will code review work with this central nav module? Do you have a subset of the team who are codeowners and are required to review / approve any changes?

[fal]

Thanks for the discussion! We'll keep this in mind going further, and revisit our assumptions. @Ryan Payne As for reviews, for "organizational" reasons (not my choice), we currently don't have code owners, but even when we did, we only had code owners for specific modules. Shared modules like what this navigation module would become, naturally wouldn't have specific code owners. I mean, the pattern at the navigation module is pretty straightforward and not unreasonably hard to enforce with custom lint rules. And also, despite the project definitely not being simple, it doesn't have 100s / 1000s of screens -- actually I can tell you, we have 59 screens. Most complexity is in the business logic.

Besides, before that, we already (unfortunately, and for other reasons) had centralized navigation on our app module before and honestly, process wise, didn't really add much overhead at all. The major difference was that before, the feature would still emit events like "NavigateToX", but the navigation handlers were all in the app module.

[ursus]

Centralization doesn’t scale, its a source of conflicts etc, don’t do it. But of course this is a function of scale, maybe you can get away with it