https://kotlinlang.org logo
Join Slack
Powered by
I'm so sad right now... With newest navigation ver...
# compose-destinations
r
I'm so sad right now... With newest navigation version, I have a lot of our extension functions shadowed by member functions 😞 I guess this is a big lesson: don't expose extension functions on your library with names that can easily be used in the future by the class they are extending. Basically, I have to completely remove these APIs, otherwise users of my library would be using the new member functions instead of mine, which obviously wouldn't work. Luckily, it will only happen on versions currently not stable, but it's still a bit annoying for everyone. Plus I have to come up with slightly different but still close enough names for these extension functions...
😬 2
😢 2
@Stylianos Gakis @Guilherme Delgado Any other idea? 😛
Example:
Copy code
Extension is shadowed by a member: public final fun <T : Any> navigate(route: T, builder: NavOptionsBuilder.() -> Unit): Unit
It's really rude 😛 All usages of this extension function would "silently" (just a warning due to experimentatl status of the new functions) start calling the new one.
New member function: (related to new type safe args APIs)
i
Note that as of the next release, the experimental tag is going to be removed from all of the new APIs, see this stack of CLs: https://android-review.googlesource.com/c/platform/frameworks/support/+/3047205
👍 1
r
Yeah... I wasn't really counting on the experimental status to be the saviour anyway 😅 But thanks for pointing that out 👍 I guess I'll have no choice but to do something weird like prefixing all these with something like "cdNavigate". The only other option would be to not expose any of the nav dependency APIs directly, and wrap everything with my own types, but that's a bit of a commitment to keep up with every change in these APIs 🤔
g
navigate
could be changed to 
navigateTo
but yeah… it’s a bit bittersweet. And I bet you have a lot more with no good prefix/suffix candidate. Well, one thing is for sure, it’s going to be a “breaking changes release”.
✅ 2
i
I know one tactic that Stylianos was using when he was playing around with the new APIs was just using Kotlin import aliasing to rename the shadowed extension method on import: https://kotlinlang.org/docs/packages.html#imports
yes black 1
r
Exactly. I liked having the same names as the official library because it makes it easier for people with official library experience. And yeah, I have stuff like 
popUpTo
(and others).
✅ 2
i
(obviously more of a work around than anything else)
r
The final result that I like the most is if I wrap the types. So instead of exposing NavController, I would expose DestinationsNavController (example). To be fair, I'm already committed to keeping up with API changes on NavController anyway. I'll think about it. Thanks guys 🙂
❤️ 1
✅ 1
i
I think if you wanted to go with a rename, I'd suggest 
navigateVia
since 'navigating via directions' reads well, but YMMV
👍 1
r
Yap.. but once again, I don't know what I'd do with other APIs like 
popUpTo
. There's also the question of the future of the library with these type safe APIs coming out. I see there's some value I can provide to users of my library because I know about the graph structure at compile time. But I suspect most people use Compose Destinations mostly for type safe args. (To be clear to anyone reading this, I plan to support the library even if I have a single user. So no one panic please 😄. I'm just writing my thoughts to also see what these smart individuals think about it 😛)
g
I do love the type safe args, no doubt, but for me, the fact that navigation graphs are very friendly to setup and the huge leverage of KSP (yup i’m lazy 🙃), boy, makes it the best nav-lib ever! IMHO 😇
👌 1
I only wish I could add valuable contributions to it 😅
r
You’re right. The way we setup the graphs with Compose Destinations requires less boilerplate compared to most alternatives. That’s another plus. I have done a very poor job in making the repo contributor friendly 😅.
g
The fact I don’t have to extend/implement from anything, it’s winner 🙏 Decompose, Voyager, don’t appeal to me because of that. I do see value in those libs, but for me it’s a turn off to begin with. And did I mention I love annotations? 😅
👍 1
s
As a trick for the client side, when I had the same issue with using compose-typed library which had the same name as the official ones, I did this https://github.com/HedvigInsurance/android/blob/1ec7b4b28e556967ab6752d45f6de1c5cf1ba136/.idea/codeInsightSettings.xml#L15-L16 This way those would just not show up on autocompletion, and my colleagues would not accidentally bring those in instead of the navigation library ones. Just a tip you may want to share in your docs too if you don’t end up disambiguating another way. And yeah alias on the import worked for my WIP but it is a very hard sell for normal users of your library.
r
I’ve used this trick internally in my work place also. (For example, we have a unique requirement to change language on the fly without leaving the app or restarting it. We made this work by having our own stringsResource functions that use our own CompositionLocal that returns resources depending on current selected language.) I think I found a good solution for my case though 🙂 We already have a DestinationsNavigator that we can use to navigate between destinations. That contains pretty much the same methods compared to the extension functions that were shadowed. So I can provide these by adding a single extension function or getter, that returns this type of navigator.
🙌 1
So people can use
Copy code
navController.navigate(Destination(args).route)
Or
Copy code
navController.navigator.navigate(Destination(args))
By doing .navigator we can access all compose destinations specific APIs.
👍 1
a
@Guilherme Delgado I might be missing something, but Decompose doesn't require you to extend/implement anything by design.
✅ 1
s
I know it’s a bit unrelated, but curious about this part
For example, we have a unique requirement to change language on the fly without leaving the app or restarting it
Isn’t that just changing the language of the app normally through 
AppCompatDelegate.setApplicationLocales()
but then also making sure you have locale change as part of the configuration change types that you handle in your manifest? Which would also then work with per-app language preferences changed from the OS settings and so on?
r
If I remember correctly, those APIs don't work for our target Android versions. Our app runs only on specialized hardware, so we have only a couple old Android versions we run on.
👍 1
s
Okay fair enough, I may be missing something for your particular requirements. We use this for all the way down to API 23 which is our min target, the code also looks to just use the platform APIs for 33 and higher but there is a fallback for older APIs too. Maybe that fallback was not something you could use then.
r
And for us, "working well with the SO" about the language is not relevant, because the hardware main purpose is to run our app. Plus we have even more difficult requirements where in one screen we may need to show certain sections in certain languages, because the device is used by two people at the same time (merchant and his customer) and they can talk different languages.
👍 1
By having a CompositionLocal, we can affect a whole branch of the compose UI tree.
So it works very well for us 🙂
s
I may be missing something for your particular requirements
Narrator: He was in fact missing more of those particular requirements
😅 1
r
How do you guys feel about WeakHashMap for performance reasons? 😛 It probably is over kill in my case, but I was coding it just to see how it would look like:
Copy code
private val navigators: WeakHashMap<NavController, DestinationsNavigator> = WeakHashMap() 
val NavController.navigator: DestinationsNavigator
    get(): DestinationsNavigator {
        return navigators[this] ?: DestinationsNavController(this)
            .also { navigators[this] = it }
    }
basically tying my navigator to the NavController, as opposed to instantiating it every time the user of the lib calls 
navigator
.
(since I have your attention 😄)
g
@Arkadii Ivanov you’re correct, I was misunderstanding the concept of the 
RootComponent
and 
ComponentContext
which btw:
…No need to extend a class from the library, or implement an interface.
😓
private val navigators: WeakHashMap<NavController, DestinationsNavigator> = WeakHashMap()
val NavController.navigator: DestinationsNavigator
get(): DestinationsNavigator {
return navigators[this] ?: DestinationsNavController(this)
.also { navigators[this] = it }
}
in which situations we end up with more than one navcontroller? 🤔 Normally I do 
val navController = engine.rememberNavController()
I may be missing something 😅
r
If you have multiple DestinationsNavHost calls.
otherwise, NavController may be GC'ed and a new one created when activity gets recreated... but that means you still only have one at a given time. ( I mean.. technically the GC may take a while to run, but there won't be any strong references to it anymore)
g
Ah of course, I do have this use-case, normally I have a Welcome/Auth screen and then a LandingActivity which will host the 
DestinationsNavHost
for the app (single activity).
I do not have both at same time, because the Welcome get’s killed after auth
but I think I get what you’re saying
If I go back to the welcome you’ll fetch from the hashmap instead of creating a new one
(if GC didn’t get it first 😅 )
r
I mean.. the goal is even valid for when there's only one NavController. I'm trying to make it so if the user of the lib does multiple 
.navigator
calls, they don't need to worry about instantiating a new instance every time. I guess if he is inside a Composable doing an animation, that could be too much? Although If inside a Composable is the only case for this, I can just use 
remember
✅ 1
s
I Havent even begun to understand Rafael's extraordinary work with destinations But i would suggest Exposing DestinationsNavController to access all the extended functions And also still provide a way to expose the internal NavController maybe if the developer just wants more control
👍 1
r
12 Views
Open in Slack
PreviousNext
Powered by