Code Monkey home page Code Monkey logo

Comments (71)

raamcosta avatar raamcosta commented on May 13, 2024 5

Hi @hrach @mhaqs @frozenjava @jahongir9779 @JohnBuhanan @llama-0 @miduch 👋

So I finally finished my changes on tivi multi-module project.
You can check the example here:
https://github.com/raamcosta/tivi

(EDIT: Forgot to mention that I used a local version of Compose Destinations so for now you guys won't be able to run this project if you clone it 😓 . I will make some more changes in the configuration after I release a new version to the public, and then you can run it (if you get the API keys needed by tivi project - description on its readme if you're interested).

A few important notes:

1- The multi-module part of it was actually mostly ok for me, meaning I didn't need anything specific to make it possible. That said, I do plan to add some further configuration such that:

  • You can easily say what kind of output you want for each module (a NavGraph, a list of destinations,...)
  • Additional more fine-grained controls like disabling the "core extensions" utilities generation (these are mostly only useful for a single module usage)

2- In the tivi project, each feature module has only 1 screen, and that screen is reused in different NavGraphs. This was actually new to me, I had never seen such an approach and it did require some additional development in Compose Destinations to make it possible (since until now, one Destination = 1 Annotated Composable = 1 unique route).
Making this possible was actually not as hard as I feared thanks to the architecture of my solution which relies heavily on abstractions.
Most of you guys don't need to mind this feature, which means you can ignore the within or routedIn extension functions I used on tivi project. But if you're curious, check the implementations of those 😉

3 - One minor annoyance is the navigating logic. There is no way for feature A to navigate directly to feature B without feature A depending on feature B module. I got around this by each module defining an interface with methods to navigate to where that module needs to navigate, then implementing all of these interfaces in a CommonNavigator of the module that calls the DestinationsNavHost (which is the module that depends on all the others).
I honestly don't really see alternatives I like better. We could go back to strings and then feature A could navigate("featureB") but I don't really like that: this is what Compose Destinations has fought to remove in the first place.
If I were developing a multi-module project, the interface approach seems more correct and it is actually not as much boilerplate as one might think. But I do hope you guys can give me feedback on that, maybe someone has a cool idea that I'm just not seeing.

4- I re-added the DependenciesContainerBuilder from a few versions back, but now improved. This allows you to set some components that will be available for all annotated Composables to use as a dependency (i.e, function parameter). I used this to make the CommonNavigator available to all destinations without having to manually call them.

Next steps:

  • Add those additional configurations so that you can configure each build.gradle of modules using the Compose Destinations KSP dependency. I expect to be done with it this weekend still 🤞
  • Add documentation on wikis to describe the new features (dynamic destinations / DependenciesContainerBuilder).
  • Releasing a new version with all the above points done. I hope I can do this by the end of this weekend or around Wednesday next week.
  • Hope you guys can try it out and give me feedback.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024 4

Hmmm yeah looking at what you said + that Chris Banes app + that KSP issue I do have some ideas..

No promises, but I'll keep you posted when I try some stuff ;)

from compose-destinations.

mhaqs avatar mhaqs commented on May 13, 2024 4

Hey thanks for the tag and putting in the effort for this to work. I'll give it a try some time on this weekend, maybe. I'll respond back with my findings.

I chose to go down the path of having a global navigation module on the root app. But I'd still like it to live on respective modules.

Thanks once again

from compose-destinations.

hrach avatar hrach commented on May 13, 2024 4

Thanks, will check in the following days.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024 3

@frozenjava
First of all, thank you! :)
I am basically expecting this to be fully working already! I just need to find the time to test it properly, but the support should be there :)
I hope next week I can get the time to fully test things and make changes if needed to improve the support.

@jahongir9779
Thank you as well! 👍

I've read in couple places where Ian Lake was stating that having multiple NavHosts is a bad thing to do in compose and by using this library in a multi module project with multiple DestinationsNavHost would be that "Bad thing" right?

Yes, multiple DestinationsNavHost is equivalent to multiple NavHost indeed. To be honest, I don't see what is the problem with having multiple of them, but I may be missing something if Ian Lake said it.
Anyway, even with a multi-modular project, you don't necessarily need multiple DestinationNavHosts. You can build a single root navigation graph and put all generated Destination in there, even ones built on different modules. You can also build nested nav graphs for each module and have them be nested in the root one you build in the module you call DestinationsNavHost. There are plenty of ways to do this.

Deep Linking and state restoration should work just like with normal compose navigation (the only difference is how you define the Deep Links which is in the annotation), even in a multi-module project.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024 3

No for now, but I am working on a fork of a known Chris Banes project: tivi (https://github.com/chrisbanes/tivi)
In the next few days, I'll be doing a few small changes on Compose Destinations Library to be easier to use in multi-module projects (you can already do it, but I want to try to make it easy).
Later I'll make my fork public so everyone can see an example of a multi-module project in practice 🙂

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024 3

awesome thanx a lot, I'll open a feature request now 😄

from compose-destinations.

mhaqs avatar mhaqs commented on May 13, 2024 2

Thanks for responding. To look at a codebase that does multi module navigation, you can look at https://github.com/chrisbanes/tivi. That's exactly the kind of set up we have for our project. I was mostly trying to find a simpler way of defining nested/complex graphs, and your plugin seemed to do just that 😄 .

The idea of having a navigation module is good, and that's what I was thinking as well. Although, that creates more code structure issues than its worth e.g. injecting deps from other modules.

I did look at some issues on the KSP repo, and found ones like this: google/ksp#372. It's really more a question of KSP being able to look at metadata from other modules. That does mean that when a module generates metadata, the classes in that metadata must be in a unique package.

So, for a quick workaround, when the compose-destinations-codegen module generates this metadata, it asks for a package location. This package location should then be used to reference where the generated code resides.

For example:

Module A

plugins {
	id(GradlePlugins.ksp).version(kspVersion)
}

composeDestinations {
  packageName.set("com.example.module.a")
}

dependencies {
	implementation (Libs.composeDestinations)
	ksp(Libs.composeDestinationsKSP)
}

Module B

plugins {
	id(GradlePlugins.ksp).version(kspVersion)
}

composeDestinations {
  packageName.set("com.example.module.b")
}

dependencies {
	implementation (Libs.composeDestinations)
	ksp(Libs.composeDestinationsKSP)
}

I haven't used KSP before, so I might just be writing nonsense. Thanks for the honest response

from compose-destinations.

mhaqs avatar mhaqs commented on May 13, 2024 2

I've tested the library with multi modules, and multi variants as well, and it does work now without changing any configurations. Thanks for putting in the effort to bring the feature 👍

from compose-destinations.

JohnBuhanan avatar JohnBuhanan commented on May 13, 2024 2

Oh nice, that will be the next piece of the puzzle needed for returning results.

And you bet.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024 1

Hi @llama-0 ! 👋

Yes, it will work just fine with Hilt, there is really no overlap between them so you won't find any issues there :)
If you find any other problem, for example with multi-module support please do let me know! I still have not had the time to test multi-module apps with this library properly, I'm thinking I will be able to very soon.

from compose-destinations.

JohnBuhanan avatar JohnBuhanan commented on May 13, 2024 1

I'm here trying to build multi-module also!

Good luck everyone :)

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024 1

Thanx a lot 🙂

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024 1

It worked! 😄 I did everything exactly as you said)) no special tweaks needed. This is the NavGraph that I've tested

object NavGraphs {

    val auth = object : NavGraphSpec {
        override val destinationsByRoute: Map<String, DestinationSpec<*>>
            get() = mapOf(Pair(AUTH_GRAPH, PhoneAuthScreenDestination))
        override val route: String
            get() = AUTH_GRAPH
        override val startRoute: Route
            get() = PhoneAuthScreenDestination
    }

    val root = object : NavGraphSpec {
        override val destinationsByRoute: Map<String, DestinationSpec<*>>
            get() = mapOf(Pair("root", SplashScreenDestination))
        override val route: String
            get() = "root"
        override val startRoute: Route
            get() = SplashScreenDestination
        override val nestedNavGraphs: List<NavGraphSpec>
            get() = listOf(auth)
    }

}

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024 1

awesome !))

from compose-destinations.

JohnBuhanan avatar JohnBuhanan commented on May 13, 2024 1

@raamcosta

I debated trying to clean this up more, but maybe getting it in front of your eyes sooner is more important.

  • Have each feature provide a "nested graph" that gets contributed to the app graph with Dagger multi-bindings.
  • Map "nested graph" and "leaf screen" destinations to simple route keys.
  • Control navigation from a ViewModel instead of from a Composable. (using simple route keys)
  • A navigation event can either be from feature to feature, or to a different screen within a feature.

https://github.com/JohnBuhanan/MVI-Public

Can you see if there is an easier way to get EntryPointActivity.Initialize to work?

Is anything about my approach salvageable?

from compose-destinations.

JohnBuhanan avatar JohnBuhanan commented on May 13, 2024 1

I'll try :P

from compose-destinations.

llama-0 avatar llama-0 commented on May 13, 2024 1

I added these lines, yes. Probably they are not recognized by some of my modules or smth. Now I'm learning to apply your lib only for 1-module project and it's working. And the next step is to expand to 2 modules in order to go slowly and hopefully by the end of the day I'll figure it out.

from compose-destinations.

miduch avatar miduch commented on May 13, 2024 1

Thanks for quick response. Applied some of your suggested changes and updated sample project.

Maybe each module should not depend on others of same level? Example: homescreen module shouldn't depend on the onboarding. I realize that this is the simplest way to navigate from homescreen to onboarding screens, but it may not be ok on most multi module projects.

Yes this is problem, any suggestions?

Oh my god sorry I wanted to give suggestion then forgot 😅 What I've done in the tivi repo is I define an interface on each module that needs to navigate with the methods it needs. For example, on "Discover feature module":

interface DiscoverNavigator {
    fun openTrendingShows()
    fun openPopularShows()
    fun openRecommendedShows()
    fun openShowDetails(showId: Long, seasonId: Long?, episodeId: Long?)
    fun openUser()
}

Then in the DestinationsNavHost call level I have a navigator that implements all those interfaces of the feature modules:

class CommonNavigator(
    private val navController: NavController,
) : RecommendedShowsNavigator,
    ShowSeasonsNavigator,
    ShowDetailsNavigator,
    DiscoverNavigator,
    TrendingShowsNavigator,
    PopularShowsNavigator,
    FollowedNavigator,
    WatchedNavigator,
    SearchNavigator,
    AccountUiNavigator,
    EpisodeDetailsNavigator {

    override fun openTrendingShows() {
        navController.navigateTo(TrendingShowsDestination)
    }
    
    //...

And finally I need to pass this navigator:

fun DestinationScope<*>.currentNavigator() = CommonNavigator(navController)

    DestinationsNavHost(
            //....
    ) {

        composable(DiscoverDestination) { Discover(currentNavigator()) }
        
        //....

thanks tomorrow i'll try to create a super simple version of it for this sample.

from compose-destinations.

miduch avatar miduch commented on May 13, 2024 1

miduch/ComposeNavigation@4c6256e
has a simple FeaturesNavigator, and it seems to work fine.

could this be done somehow at level when i'm setting up NavGraphSpec and probably with some support from library to generate needed stuff, i could have avoid manual composable(....) calls?

from compose-destinations.

llama-0 avatar llama-0 commented on May 13, 2024 1

Hi @raamcosta! Thanks for the active development. I had no chance to go through the recent changes thoroughly, but I wonder whether you saw this article on having api & impl modules for each feature? It may help with navigation from featureA to featureB. I hope this will be useful.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024 1

Tivi also now is working:
https://github.com/raamcosta/tivi

Besides, and just to reiterate, you can also check this app from Philipp Lackner multi module app:
https://github.com/raamcosta/CalorieTracker

Already two sample multi module apps! (Both still using a snapshot version, still need to add documentation and release it)
It would be great if you guys could check them and give me feedback 🙂

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024 1

Hi guys! New release 1.3.1-beta with the new configurations to make this easier.
Let me know if it works for you. Both tivi and CalorieTracker are examples of different approaches on multi module projects regarding navigation.

Read more about the new configs here https://github.com/raamcosta/compose-destinations/releases/tag/1.3.1-beta

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Hi @mhaqs ! 🙂

Can you tell us a little bit more about your setup? Do modules A and B both use Compose Destinations library? Do both use @Destination annotations?
Can you show us the different build.gradle files?

from compose-destinations.

mhaqs avatar mhaqs commented on May 13, 2024

Yeah, both modules use compose-destinations . Both modules have this in the build.gradle file.

plugins {
	id(GradlePlugins.ksp).version(kspVersion)
}

dependencies {
	implementation (Libs.composeDestinations)
	ksp(Libs.composeDestinationsKSP)
}

Both modules are then included in the app module which has the generatedPath snippet for all variants. I have tried it with local path additions in the module build gradle files as well, but it doesn't work either way.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Ok thanks.

So honestly this is not something I've thought about before. But for Compose Destinations to work, we would need a central place (in your case the app module) to generate all the files. The problem is that the annotations are spread in modules A and B and there is no way (that I know of) of making KSP process annotations from other modules.

So I don't think this is possible, at least not without some serious work and thought. And for now, I am just a guy maintaining the library and I have a job too, so there is just no way. (But I am looking for people to help me, in case you're interested 😁)

All that said, I'd like to take a look in a code base like the one you're trying to build just with normal compose navigation. Just to see how it would work, maybe this gives me some ideas.

All I can think of is preparing the destinations all in the same module (your app module or just create a "navigation" module) then each would call other Composables from module A and B. This way navigation logic would also be all in one place, but then the actual Composables which compose actual UI would be in the feature modules.
If it's really necessary to have the @destination in different modules, then Compose Destinations is not for you atm 😢

Sorry for the long answer 🙂

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

In the mean time, my earlier comment is still valid:
You can use @destination only on your app module, deal with all navigation stuff there, then just call composables of your feature modules.
This is actually the equivalent of what Chris Banes is doing in his app 🙂

from compose-destinations.

mhaqs avatar mhaqs commented on May 13, 2024

Yeah, I'll try a few things and see how it turns out. Mostly that I can have a navigation module just below the app module which includes all feature modules and deals with destinations.

from compose-destinations.

hrach avatar hrach commented on May 13, 2024

Hi, my two bits how this could work:

  • have a ksp processor & core dependency in all feature modules
  • the generated code will contains all destinations and a variable containing module's List<Destination>
  • app main module uses core dependency and creates the NavHost and passes the list of all destinations:
package featureA

val destinations: List<Destination> = listOf(...)
Destinations.NavHost(featureA.destinations + featureB.destinations)

The main difference to current solution is:

  • the NavHost requires explicit list of destinations (possible could have default = the current module's destination).
  • CoreExtensions & DestinationsNavHost should be rather in the core package and not a generated code.

The actual destination sharing is an implementation detail and it is up to the implementator - i.e. the list of destinations may stay "hidden" and developer may utilize Dependency Injection and use provider/factory pattern (for both list of destinations & and factory for Routed instance)

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

EDIT: Outdated:

Just wanna add that the reason why CoreExtensions and DestinationsNavHost are generated is because they adapt to the dependencies the user is using (Accompanist stuff).

To put them in the core, I'd have to publish more artifacts and even then it would not be straightforward.
The other reason is the sealed Destination feature.

I think it is possible to configure my ksp processor at gradle level to generate them only in one of the modules (the one that would call DestinationsNavHost) and all others would just generate the Destination objects.

Other than that, you have some good ideas there :)

I honestly don't know when I'll go down this rabbit hole, I still like the approach in my previous comment. Marking Composables as @Destination makes sense to be done always in the same module, because just by doing that you're making a navigation decision. And I think it makes sense that all those are done in the same module. Feature modules should focus on displaying the UI. Then your annotated Composables would just call the feature module ones. Maybe take advantage of the annotated Composable to prepare additional stuff for the feature module Composable (example create the ViewModel and pass lambdas for UI Events and UI State classes and just pass those to the feature module Composable).

If we take a look at the Chis Banes example, we will see that that is actually what he's doing (without Compose Destinations of course).

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Hi @mhaqs and @hrach 👋

So good news, with version 1.1.1-beta, Destination generated classes will be put by default on a package with the common prefix from all @destination annotated Composable functions.
Besides, you can change this explicitly with:

ksp {
    // To turn off navigation graphs generation:
    // (if you prefer creating NavGraph instances or you prefer using the normal NavHost)
    arg("compose-destinations.generateNavGraphs", "false")

    // To change the package name where the generated files will be placed
    arg("compose-destinations.codeGenPackageName", "your.preferred.packagename")
}

Note also the first configuration, which is probably also a good thing to do for multi-module projects.
I haven't had the time to try this yet, but, if I'm right, with these configs, each module can generate its own Destination files, and then the module that calls DestinationsNavHost can build a NavGraphSpec class and pass it to the DestinationsNavHost call.

With these changes and much more features/improvements that I implemented in the library since our discussion, I'd be thrilled if any one of you could try this setup 🙂
I also will make the time some day (hopefully soon) to set up a multi-module project to try it (or who knows maybe I'll make a PR on Chris Banes tivi repo 😄 ).

Thank you guys once again, and hope to hear from you soon!

Rafael Costa

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Cool thanks for letting me know. Btw I'm curious on how you're setting up the navi graphs. You mentioned that you didn't need any configuration.
Does that mean that each module is creating it's own nav graph? Or are you ignoring the auto generated Na graphs objects and creating them yourself?

Thanks in advance!

from compose-destinations.

mhaqs avatar mhaqs commented on May 13, 2024

Every module has its own graph indeed, and their own start destinations. The app has isolated zones which do not require traversing other graphs once you're past using them. This keeps the graph groups clean. We just group all destinations together, and when the user moves to the next graph, the start destination is changed as well.

It's complex, and that's why organising the destination logic required that we don't have something global. But, even with a root and nested graph structure, the isolated nav graph generation should still be ok, as long as your root graph knows how to traverse them. That's what tivi app does, and that's what you touch on in the wiki here: https://github.com/raamcosta/compose-destinations/wiki/Defining-your-navigation-graphs#manually-defining-navigation-graphs.

Moreover, the usability of a library like Compose-Destinations is reduced greatly when you have to manually manage the NavHosts and pass a lot of information/states down the graph. It'll be up to anyone going this route to weigh out, if it's worth investing time to let a library generate the boilerplate or do it manually.

from compose-destinations.

frozenjava avatar frozenjava commented on May 13, 2024

Hello. I don't have anything else to add to this at the moment. Just wanted to say I also have a few apps that are multi-modular following the same pattern mentioned above and I am looking into using this library as well.

If there are two of us commenting there are probably more out there so it might be a use-case that should be kept in mind.

Great work so far on the library, really makes life easier!

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024

Hi, thanx for a cool lib, I just wanted to know if anyone tested the deeplinking and state restoration in a multimodule project with this lib... I've read in couple places where Ian Lake was stating that having multiple NavHosts is a bad thing to do in compose and by using this library in a multi module project with multiple DestinationsNavHost would be that "Bad thing" right? Would really appreciate some clarifications on this issue please...

from compose-destinations.

llama-0 avatar llama-0 commented on May 13, 2024

@mhaqs & @hrach Hi there! I'm about to start building a multi-module application in compose. In my prerequisite research, I stumbled on this discussion (thanks a lot for that, btw) and now I wonder whether any of you used Hilt as a DI framework in your projects? Does it work well with this library?

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

@JohnBuhanan I am at risk of sounding repetitive here but please let us know how that goes! 🙂

I am currently finishing support for passing arguments to back destination in a type-safe and easier way.
After I release that I'll start looking into a multi-module sample.

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024

Hi, I've tried it in a multi module project. Tried both with DestinationsNavHost and NavHost with my own NavGraph classes (by setting the arg("compose-destinations.generateNavGraphs", "false")) but both approaches ended with:

DexArchiveMergerException: Error while merging dex archives: ... ...DestinationKt is defined multiple times

do you have any ideas on what I might be doing wrong?

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

By default, the package name used for all generated files will be the common part of all package names of the annotated Composables.
It seems to me like in your case multiple modules are using the same package name. If so, you can explicitly ask for a package name where all generated files will live by module.

On each module's build.gradle where you use compose destinations, you would add:

ksp {
    arg("compose-destinations.codeGenPackageName", "your.package.name.modulea")
}

replacing modulea with the name of that module.

Let me know if this helped :)

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024

not sure if my understandings are right, please correct me if I'm wrong...

I'm trying to have multiple NavGraphs within a Single DestinationsNavHost. I have different feature modules with screens that need to be put inside a separate NavGraphs. To achieve this:

  1. I need to set compose-destinations.codeGenPackageName to a common module while allowing NavGraph generation.
  2. Assigning navGraph param for each @Destination inside a NavGraph with only a single start = true screen
  3. Include all of the @destinations in a composable(..){} inside DestinationNavHost
  4. Navigate to a different navGraph by destinationsNavigator.navigate(NavGraphs.[yourNavGraphName])

does these steps sound right?

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Hmm..
It really depends on your setup, I'd need more specifics.
But the most consistent way of doing this would be each feature module should do this:

ksp {
    arg("compose-destinations.generateNavGraphs", "false") // after this, you should not use "navGraph" or "start" on annotation
    arg("compose-destinations.codeGenPackageName", "your.packagename.featureA") // replacing featureA with the feature name
}

Then in your navigation module where you call DestinationsNavHost, also disable nav graph generation and create an object like this:

object NavGraphs {

    val featureA = NavGraph(//... in the list of destinations, use all generated destinations of module "featureA")
    //...
    val featureN = NavGraph//... one of this per nested navigation graph
    //...
   
    val root = NavGraph(
        startRoute = featureX, // use the feature where your app starts
        //... leave list of destinations empty (probably?) then in "nestedNavGraphs" get all the above nav graphs, so:
        nestedNavGraphs = listOf(featureA, featureB, /*...*/, featureN)
    )
}

You then pass that NavGraphs.root to the DestinationsNavHost call. You don't need to add any composable call inside DestinationsNavHost unless you really need to pass some component that the library can't provide.

To navigate you are on point. But that will only work from your navigation module where you defined the above NavGraphs. If you need to navigate from featureA to featureB, one way to do it is that featureA declares some interface with the navigations it needs to do, then this common navigation module would provide an implementation where it can use the feature NavGraphs above.

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024

Thank you for your time, I've tried what you advised, but now having ClassCastException: ...Destination cannot be cast to packagename.destinations.TypedDestination

because the screens in different modules are using their own generated Destination.kt

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

You're right 🤔
I should make clear that this support is still very experimental, I am now on process to start experimenting with it and later on there will be probably added features to make all this easier.

In the mean time, you can solve this by just using DestinationSpec instead of Destination in the NavGraph data class.
Basically, in your NavGraphs object, replace instantiating the generated NavGraph with using NavGraphSpec directly, so:

object NavGraphs {

    val featureA = object: NavGraphSpec {(//... in the list of destinations, use all generated destinations of module "featureA")
    //...
    val featureN = object: NavGraphSpec {//... one of this per nested navigation graph
    //...
   
    val root = object: NavGraphSpec {
        startRoute = featureX, // use the feature where your app starts
        //... leave list of destinations empty (probably?) then in "nestedNavGraphs" get all the above nav graphs, so:
        nestedNavGraphs = listOf(featureA, featureB, /*...*/, featureN)
    )
}

All untested code, but this would be the idea.
This should work because all generated Destination implement the same DestinationSpec.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Let me know if you get that to work. Are you working on something we can see the code? Is it open source? 🙂

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Cool! A way to simplify that code:

object NavGraphs {

    val auth = object : NavGraphSpec {
        override val route = AUTH_GRAPH
        override val destinationsByRoute = listOf(PhoneAuthScreenDestination).associateBy { it.route }
        override val startRoute = PhoneAuthScreenDestination
    }

    val root = object : NavGraphSpec {
        override val route = "root"
        override val destinationsByRoute = listOf(SplashScreenDestination).associateBy { it.route }
        override val startRoute = SplashScreenDestination
        override val nestedNavGraphs = listOf(auth)
    }

}

Note that the route of each destination must not equal the route of the NavGraph itself. The associateBy is your friend here: you just need to make a list of Destinations.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Is there a way for us to connect without spamming this thread? I'm on twitter if you can DM me. Because this doesn't seem as easy to answer as other questions 😄
https://twitter.com/raamcosta

from compose-destinations.

miduch avatar miduch commented on May 13, 2024

is there a simple sample available for multi module?

from compose-destinations.

llama-0 avatar llama-0 commented on May 13, 2024

It would be great if someone, who succeeded in applying your lib to the multi-module project will share a basic example because the configuration is a pain. I still can't manage to generate NavGraphs -- the build/generated/ksp/debug folder is empty (no kotlin folder present)

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Are you looking at the folder from the IDE? If yes, check the folder in the terminal/explorer. Because you might be missing the last step of the README:

kotlin {
    sourceSets {
        debug {
            kotlin.srcDir("build/generated/ksp/debug/kotlin")
        }
        release {
            kotlin.srcDir("build/generated/ksp/release/kotlin")
        }
    }
}

This (or similar) needs to apply to each module.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

You need those lines for each module's build.gradle.
So yeah copy paste those into the build.gradle of each module 🙂

from compose-destinations.

miduch avatar miduch commented on May 13, 2024

here is super simple example @llama-0, seems to work fine

https://github.com/miduch/ComposeNavigation/raw/master/screenshots/composenavigation.mp4

https://github.com/miduch/ComposeNavigation

hopefully @raamcosta can point out how this could be done better :)

not sure how to have onboarding route shown only once upon first run, and after that always homescreen route is root? now when auto launched from homescreen route, there is some flicker happening

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

@miduch Things to improve:

  • You should probably use this on all modules since you are manually creating your own NavGraphs on AppNavigation.kt
ksp { //you already have ksp block, just need this next line on all of them:
    arg("compose-destinations.generateNavGraphs", "false")
}

After this you can remove the "start = true" from all your screens.

  • Maybe each module should not depend on others of same level? Example: homescreen module shouldn't depend on the onboarding. I realize that this is the simplest way to navigate from homescreen to onboarding screens, but it may not be ok on most multi module projects.
  • I'm not sure where you're using these:
    implementation 'com.google.accompanist:accompanist-navigation-animation:0.22.0-rc'
    implementation 'com.google.accompanist:accompanist-navigation-material:0.22.0-rc'

But Compose Destinations has good integration with them via "animations-core" instead of "core" dependency.

not sure how to have onboarding route shown only once upon first run, and after that always homescreen route is root? now when auto launched from homescreen route, there is some flicker happening

You can pass a startRoute to DestinationsNavHost call, overriding the startRoute set in the NavGraphSpec of your NavGraph.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Other than that, it is a nice sample. Thank you @miduch 👏

from compose-destinations.

miduch avatar miduch commented on May 13, 2024

Thanks for quick response. Applied some of your suggested changes and updated sample project.

Maybe each module should not depend on others of same level? Example: homescreen module shouldn't depend on the onboarding. I realize that this is the simplest way to navigate from homescreen to onboarding screens, but it may not be ok on most multi module projects.

Yes this is problem, any suggestions?

You can pass a startRoute to DestinationsNavHost call, overriding the startRoute set in the NavGraphSpec of your NavGraph.

Did you mean something like this?

fun AppNavigation() {
    val showOnboarding = listOf(true, false).random()
    DestinationsNavHost(
        navGraph = AppNavGraphs.home,
        startRoute = if (showOnboarding) AppNavGraphs.onboarding.startRoute else AppNavGraphs.home.startRoute
    )
}

it gives me java.lang.IllegalArgumentException: navigation destination on_boarding_screen is not a direct child of this NavGraph

from compose-destinations.

miduch avatar miduch commented on May 13, 2024

following seems to launch onboarding screen without that flicker

@Composable
fun AppNavigation() {
    val showOnboarding = listOf(true, false).random()
    if (showOnboarding) {
        DestinationsNavHost(
            navGraph = AppNavGraphs.home,
            startRoute = AppNavGraphs.onboarding
        )
    } else {
        DestinationsNavHost(navGraph = AppNavGraphs.home)
    }
}

but now navigator.popBackStack() doesn't work anymore from OnBoardingScreen when done button is pressed.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Try this:

fun AppNavigation() {
    val showOnboarding = listOf(true, false).random()
    DestinationsNavHost(
        navGraph = AppNavGraphs.home,
        startRoute = if (showOnboarding) AppNavGraphs.onboarding else AppNavGraphs.home
    )
}

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Thanks for quick response. Applied some of your suggested changes and updated sample project.

Maybe each module should not depend on others of same level? Example: homescreen module shouldn't depend on the onboarding. I realize that this is the simplest way to navigate from homescreen to onboarding screens, but it may not be ok on most multi module projects.

Yes this is problem, any suggestions?

Oh my god sorry I wanted to give suggestion then forgot 😅
What I've done in the tivi repo is I define an interface on each module that needs to navigate with the methods it needs.
For example, on "Discover feature module":

interface DiscoverNavigator {
    fun openTrendingShows()
    fun openPopularShows()
    fun openRecommendedShows()
    fun openShowDetails(showId: Long, seasonId: Long?, episodeId: Long?)
    fun openUser()
}

Then in the DestinationsNavHost call level I have a navigator that implements all those interfaces of the feature modules:

class CommonNavigator(
    private val navController: NavController,
) : RecommendedShowsNavigator,
    ShowSeasonsNavigator,
    ShowDetailsNavigator,
    DiscoverNavigator,
    TrendingShowsNavigator,
    PopularShowsNavigator,
    FollowedNavigator,
    WatchedNavigator,
    SearchNavigator,
    AccountUiNavigator,
    EpisodeDetailsNavigator {

    override fun openTrendingShows() {
        navController.navigateTo(TrendingShowsDestination)
    }
    
    //...

And finally I need to pass this navigator:

fun DestinationScope<*>.currentNavigator() = CommonNavigator(navController)

    DestinationsNavHost(
            //....
    ) {

        composable(DiscoverDestination) { Discover(currentNavigator()) }
        
        //....

from compose-destinations.

miduch avatar miduch commented on May 13, 2024

val showOnboarding = listOf(true, false).random()
DestinationsNavHost(
navGraph = AppNavGraphs.home,
startRoute = if (showOnboarding) AppNavGraphs.onboarding else AppNavGraphs.home
)

have already tried this and i get java.lang.IllegalArgumentException: Start destination homescreen_route cannot use the same route as the graph NavGraph(0xd6b098a2) route=homescreen_route startDestination=0x0
when onboarding is false

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Ahh I guess it's because when onboarding is false, you are passing a start destination equal to the navgraph your're using :P

Try this:

fun AppNavigation() {
    val showOnboarding = listOf(true, false).random()
    DestinationsNavHost(
        navGraph = AppNavGraphs.home,
        startRoute = if (showOnboarding) AppNavGraphs.onboarding else AppNavGraphs.home.startRoute
    )
}

from compose-destinations.

miduch avatar miduch commented on May 13, 2024

Ahh I guess it's because when onboarding is false, you are passing a start destination equal to the navgraph your're using :P

Try this:

fun AppNavigation() {
    val showOnboarding = listOf(true, false).random()
    DestinationsNavHost(
        navGraph = AppNavGraphs.home,
        startRoute = if (showOnboarding) AppNavGraphs.onboarding else AppNavGraphs.home.startRoute
    )
}

yes this works fine.

One remaining issue is how to get rid of onboarding after its shown. When done button is pressed i have tried following

onClick = { navigator.popBackStack() } -> closes onboarding screen, but homescreen isn't shown, just some white/blank screen

onClick = { navigator.navigate(HomeScreenDestination) } -> closes onboarding screen, homescreen is shown, but onboarding is still in backstack (pressing back button from homescreen will show onboarding again)

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

These are all questions you'd have to answer with Jetpack Compose Navigation too btw. And the answer is the same as well. That's a good advantage of Compose Destinations, most knowledge going around for Official Jetpack Compoe Navigation applies here too.
Just to make it clear 😄
But this is what you want to do:

navigator.navigate(HomeScreenDestination) {
    popUpTo(OnboardingScreenDestination.route) {
        inclusive = true
    }
}

Let me know if that helps 😉

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Additional help from the library is still something I am exploring at the moment.
But most likely, yes there will be a way to give dependencies to the lib so the lib can give them to Composables automatically 🙂

from compose-destinations.

developersancho avatar developersancho commented on May 13, 2024

Thank you very much your example of multi-module project. I downloaded tivi project. When i sync i got this error "Failed to resolve: io.github.raamcosta.compose-destinations:animations-core:1.2.3-beta"

Actually In the repo last release version is 1.2.2-beta

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Hi @developersancho
Yap, forgot to mention, I'm using a snapshot version which is not yet available 😅
Anyway, the project needs 2 keys from specific APIs to actually work, so I was guessing people would use it mainly to check the code and less to actually run it.
But I should have definitely said it, my bad 😅

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Thanks @llama-0 I believe I skimmed through that article but I will read it with more care now.
From what I remember, it seems like a bunch of good practices related to the implementation details of a multi-module project. So I hope my library works well with those practices, but they will probably not influence my library APIs decision.

That said, I will read the article carefully as I said 👍

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

This got a mention on PL-coding calories tracker app, so I did the changes for multi module:

https://github.com/raamcosta/CalorieTracker

This time I used jitpack as a SNAPSHOT repository, so you guys can actually build the app and see it working! 🙂

@developersancho tagging you since you wanted to try tivi project. I can also do the same in tivi project and use this snapshot. Will do that change later today.

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024

Hi, I'm having Non existent Destination ('ErrorBottomSheetDestination') as the ResultRecipient's result origin! error for a DestinationStyle.BottomSheet destination. Can you please help?

@Composable
@Destination(style = DestinationStyle.BottomSheet::class)
fun ErrorBottomSheet(
    title: String,
    message: String,
    buttonText: String,
    resultNavigator: ResultBackNavigator<Boolean>
) {...

I have added the generated destination to the destinationsByRoute field of each NavGraphSpec . The dialog lives in a different module from where the resultRecipient() is being passed.

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

Yeah.. result back feature will not work if the Result origin and Result recipient are not in the same module 🙁
You have to fall back to normal previousBackStackEntry (you can look at the implementation of ResultBackNavigator and ResultRecipient for clues).

In your case the recipient Destination knows about this Destination? I.e, does the module of the recipient depend on the module of the result origin?

from compose-destinations.

jmadaminov avatar jmadaminov commented on May 13, 2024

yes the recipient module in my case app module depends on the module where ErrorBottomSheet is. The idea I had is, to be able to use Dialogs from any feature modules with result callback...

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

This is not possible at the moment. But if the recipient module depends on the sender, then I believe I could probably make it work 🤔 I'm not 100% sure.. KSP would have to be able to query for files of other modules and it would have to run after those other modules were already done with their KSP task.

Can you please open a feature request for it? I'll look into it when I have the time.
In the meantime, you have to use navController and previousBackStackEntry, if you really want to send results back. It won't be as clean and type-safe, but at least you're not blocked I guess 🙂
Thank you!

from compose-destinations.

raamcosta avatar raamcosta commented on May 13, 2024

I will close this issue, it is long enough already and I think for the most part we have official support in the library good enough for me to be able to say that we have multi-module support.

Additional suggestions, improvements or different multi-module setups that the support can't deal with very well, please open a new issue.

Thank you all for being part of the discussion 🙇
I really appreciate every single one of you guys!

from compose-destinations.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.