Open
Conversation
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
Increases consistency feature/template-194-navigation3
ninovanhooff
commented
Jan 9, 2026
feature/template-194-navigation3
…94-navigation3 # Conflicts: # app/src/main/kotlin/nl/q42/template/MainActivity.kt # gradle/libs.versions.toml
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
feature/template-194-navigation3
There was a problem hiding this comment.
Pull request overview
This pull request implements a migration from Navigation Compose 2.x to Navigation 3, a major refactoring of the app's navigation system. The migration introduces a custom navigation state management solution that avoids tight coupling with Koin's Navigator, providing more flexibility while maintaining the existing navigation patterns. The PR includes comprehensive deeplink support using Navigation 3's serialization-based approach.
Changes:
- Migrated from androidx.navigation:navigation-compose to androidx.navigation3 libraries (runtime and ui)
- Replaced navigation graphs with entry provider scopes and custom Navigator class for back stack management
- Implemented deeplink parsing infrastructure with pattern matching and key decoding
- Updated ViewModels to receive Destination parameters via Koin's parametersOf mechanism
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Updated Koin to 4.2.0-RC1, added Navigation 3 libraries, added koin-annotations, removed old composeNavigation dependency |
| feature/home/src/main/kotlin/nl/q42/template/home/second/presentation/HomeSecondViewModel.kt | Replaced SavedStateHandle with @provided Destination.HomeSecond parameter |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/viewmodel/RouteNavigator.kt | Renamed navigationState to appNavigationState and NavigationState to AppNavigationState |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/viewmodel/Navigator3.kt | New Navigator class that manages navigation by manipulating back stacks directly |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/viewmodel/NavigationState.kt | Complete rewrite - now manages Navigation 3 back stacks and converts them to NavEntries |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/viewmodel/InitNavigator.kt | Updated to work with new Navigator class instead of NavHostController |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/viewmodel/AppNavigationState.kt | New file containing the old NavigationState and BackstackBehavior definitions |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/Destinations.kt | Made Destination sealed class extend NavKey interface |
| core/navigation/src/main/kotlin/nl/q42/template/navigation/AppGraphRoutes.kt | Removed file - no longer needed with Navigation 3's entry-based approach |
| build.dep.navigation.gradle | Updated to use Navigation 3 libraries |
| build.dep.di.gradle | Added koin-annotations dependency |
| app/src/main/kotlin/nl/q42/template/navigation/deeplink/KeyDecoder.kt | New decoder for deserializing deeplink arguments into Destination objects |
| app/src/main/kotlin/nl/q42/template/navigation/deeplink/DeepLinkRequest.kt | New class to parse and store deeplink URIs in a structured format |
| app/src/main/kotlin/nl/q42/template/navigation/deeplink/DeepLinkPattern.kt | New class to parse and validate deeplink patterns against Destination schemas |
| app/src/main/kotlin/nl/q42/template/navigation/deeplink/DeepLinkParser.kt | New parser that matches incoming deeplink intents to registered patterns |
| app/src/main/kotlin/nl/q42/template/navigation/deeplink/DeepLinkMatcher.kt | New matcher that compares requested deeplinks against supported patterns |
| app/src/main/kotlin/nl/q42/template/navigation/OnboardingDestinations.kt | Converted from NavGraphBuilder extension to EntryProviderScope extension |
| app/src/main/kotlin/nl/q42/template/navigation/HomeGraph.kt | Removed - replaced by HomeEntry.kt |
| app/src/main/kotlin/nl/q42/template/navigation/HomeEntry.kt | New file defining home destination entries using Navigation 3 patterns |
| app/src/main/kotlin/nl/q42/template/di/AppModule.kt | Added DeeplinkParser as singleton |
| app/src/main/kotlin/nl/q42/template/MainActivity.kt | Replaced NavHost with NavDisplay and integrated deeplink parsing at startup |
| app/build.gradle | Removed old composeNavigation dependency |
| .idea/copilotDiffState.xml | IDE-specific file that should not be in version control |
Files not reviewed (1)
- .idea/copilotDiffState.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| InitNavigator(navController = navController, viewModel) | ||
| internal fun EntryProviderScope<NavKey>.onboardingEntry(navigator: Navigator) { | ||
| entry<Destination.Onboarding> { key -> | ||
| val viewModel: OnboardingStartViewModel = koinViewModel { parametersOf(key) } |
There was a problem hiding this comment.
Similar to HomeViewModel, OnboardingStartViewModel does not accept a Destination.Onboarding parameter but parametersOf(key) is being passed during its construction. This will cause a DI injection failure at runtime. Either remove the parametersOf(key) call or add a @Provided params: Destination.Onboarding parameter to OnboardingStartViewModel if the destination data is needed.
Suggested change
| val viewModel: OnboardingStartViewModel = koinViewModel { parametersOf(key) } | |
| val viewModel: OnboardingStartViewModel = koinViewModel() |
| PrimitiveKind.INT -> String::toInt | ||
| PrimitiveKind.BOOLEAN -> String::toBoolean | ||
| PrimitiveKind.BYTE -> String::toByte | ||
| PrimitiveKind.CHAR -> String::toCharArray |
There was a problem hiding this comment.
The PrimitiveKind.CHAR type parser returns String::toCharArray which produces a CharArray, not a Char. This will cause a type mismatch when trying to decode a character parameter. The correct parser should be { it.single() } to get a single Char from the string.
Suggested change
| PrimitiveKind.CHAR -> String::toCharArray | |
| PrimitiveKind.CHAR -> { it.single() } |
| @@ -1,3 +1,4 @@ | |||
| dependencies { | |||
| implementation(libs.koin) | |||
| implementation(libs.koin.annotations) | |||
There was a problem hiding this comment.
The koin-annotations library is being added to the dependencies but it's only used in one place (@Provided annotation in HomeSecondViewModel). Additionally, the @Provided annotation usage appears to be incorrect - this annotation is part of Koin's code generation system which requires additional setup with KSP processor. Without the processor configured, this annotation won't work as intended. Consider either properly setting up Koin annotations with KSP, or using the standard Koin parametersOf mechanism without the annotation.
Suggested change
| implementation(libs.koin.annotations) |
| navController.navigateUp() | ||
| onNavigated(navigationState) | ||
| is AppNavigationState.NavigateUp -> { | ||
| navigator.goBack() |
There was a problem hiding this comment.
The NavigateUp case is missing a call to onNavigated(appNavigationState) after calling navigator.goBack(). This inconsistency with the other navigation cases means the navigation state won't be properly reset to Idle after a back navigation, potentially causing the same navigation action to be triggered multiple times.
Suggested change
| navigator.goBack() | |
| navigator.goBack() | |
| onNavigated(appNavigationState) |
Comment on lines
+1
to
+18
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project version="4"> | ||
| <component name="CopilotDiffPersistence"> | ||
| <option name="pendingDiffs"> | ||
| <map> | ||
| <entry key="$PROJECT_DIR$/gradle/libs.versions.toml"> | ||
| <value> | ||
| <PendingDiffInfo> | ||
| <option name="filePath" value="$PROJECT_DIR$/gradle/libs.versions.toml" /> | ||
| <option name="originalContent" value="[versions] jvmTarget = "21" kotlin = "2.2.20" # should match kotlin https://github.com/google/ksp/releases gradlePlugin = "8.11.2" googleServices = "4.4.4" crashlyticsPlugin = "3.0.6" firebaseBOM = "34.4.0" retrofit = "3.0.0" kotlinx-serialization = "1.9.0" retrofit2KotlinxSerializationConverter = "1.0.0" networkResponseAdapter = "5.0.0" napier = "2.7.1" composeNavigation = "2.9.5" okhttp = "5.2.1" composePlatform = "2025.10.00" activityCompose = "1.11.0" composeLifecycle = "2.9.4" # Test dependencies kotlinxCoroutinesTest = "1.10.2" junit = "4.13.2" mockkAndroid = "1.14.6" turbine = "1.2.1" composeStateEvents = "2.2.0" koin = "4.1.1" [libraries] junit = { module = "junit:junit", version.ref = "junit" } kotlin-test = { module = "org.jetbrains.kotlin:kotlin-test" } kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinxCoroutinesTest" } mockk-agent = { module = "io.mockk:mockk-agent", version.ref = "mockkAndroid" } mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockkAndroid" } retrofit = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" } retrofit2-kotlinx-serialization-converter = { module = "com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter", version.ref = "retrofit2KotlinxSerializationConverter" } kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinx-serialization" } networkResponseAdapter = { module = "com.github.haroldadmin:NetworkResponseAdapter", version.ref = "networkResponseAdapter" } okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" } okhttpLogging = { module = "com.squareup.okhttp3:logging-interceptor", version.ref = "okhttp" } napier = { module = "io.github.aakira:napier", version.ref = "napier" } composeUIGraphics = { module = "androidx.compose.ui:ui-graphics" } composeUITooling = { module = "androidx.compose.ui:ui-tooling" } composeUIToolingPreview = { module = "androidx.compose.ui:ui-tooling-preview" } composeMaterial3 = { module = "androidx.compose.material3:material3" } composePlatform = { module = "androidx.compose:compose-bom", version.ref = "composePlatform" } firebaseBoM = { module = "com.google.firebase:firebase-bom", version.ref = "firebaseBOM" } firebaseCrashlytics = { module = "com.google.firebase:firebase-crashlytics" } activityCompose = { module = "androidx.activity:activity-compose", version.ref = "activityCompose" } composeLifecycle = { module = "androidx.lifecycle:lifecycle-runtime-compose", version.ref = "composeLifecycle" } turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" } composeStateEvents = { module = "com.github.leonard-palm:compose-state-events", version.ref = "composeStateEvents" } composeNavigation = { module = "androidx.navigation:navigation-compose", version.ref = "composeNavigation" } koin = { module = "io.insert-koin:koin-android", version.ref = "koin" } koin-compose = { module = "io.insert-koin:koin-androidx-compose", version.ref = "koin" } koin-test = { module = "io.insert-koin:koin-test-junit4", version.ref = "koin" } [plugins] androidApplication = { id = "com.android.application", version.ref = "gradlePlugin" } androidLibrary = { id = "com.android.library", version.ref = "gradlePlugin" } kotlinSerialization = { id = "org.jetbrains.kotlin.plugin.serialization", version.ref = "kotlin" } jetbrainsKotlinAndroid = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" } googleServices = { id = "com.google.gms.google-services", version.ref = "googleServices" } firebaseCrashlyticsPlugin = { id = "com.google.firebase.crashlytics", version.ref = "crashlyticsPlugin" } compose-compiler = { id = "org.jetbrains.kotlin.plugin.compose", version.ref = "kotlin" } " /> | ||
| <option name="updatedContent" value="[versions] jvmTarget = "21" kotlin = "2.2.20" # should match kotlin https://github.com/google/ksp/releases gradlePlugin = "8.11.2" googleServices = "4.4.4" crashlyticsPlugin = "3.0.6" firebaseBOM = "34.4.0" retrofit = "3.0.0" kotlinx-serialization = "1.9.0" retrofit2KotlinxSerializationConverter = "1.0.0" networkResponseAdapter = "5.0.0" napier = "2.7.1" composeNavigation = "2.9.5" okhttp = "5.2.1" composePlatform = "2025.10.00" activityCompose = "1.11.0" composeLifecycle = "2.9.4" # Test dependencies kotlinxCoroutinesTest = "1.10.2" junit = "4.13.2" mockkAndroid = "1.14.6" turbine = "1.2.1" composeStateEvents = "2.2.0" koin = "4.1.1" [libraries] junit = { module = "junit:junit", version.ref = "junit" } kotlin-test = { module = "org.jetbrains.kotlin:kotlin-test" } kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinxCoroutinesTest" } mockk-agent = { module = "io.mockk:mockk-agent", version.ref = "mockkAndroid" } mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockkAndroid" } retrofit = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" } retrofit2-kotlinx-serialization-converter = { module = "com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter", version.ref = "retrofit2KotlinxSerializationConverter" } kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinx-serialization" } networkResponseAdapter = { module = "com.github.haroldadmin:NetworkResponseAdapter", version.ref = "networkResponseAdapter" } okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" } okhttpLogging = { module = "com.squareup.okhttp3:logging-interceptor", version.ref = "okhttp" } napier = { module = "io.github.aakira:napier", version.ref = "napier" } composeUIGraphics = { module = "androidx.compose.ui:ui-graphics" } composeUITooling = { module = "androidx.compose.ui:ui-tooling" } composeUIToolingPreview = { module = "androidx.compose.ui:ui-tooling-preview" } composeMaterial3 = { module = "androidx.compose.material3:material3" } composePlatform = { module = "androidx.compose:compose-bom", version.ref = "composePlatform" } firebaseBoM = { module = "com.google.firebase:firebase-bom", version.ref = "firebaseBOM" } firebaseCrashlytics = { module = "com.google.firebase:firebase-crashlytics" } activityCompose = { module = "androidx.activity:activity-compose", version.ref = "activityCompose" } composeLifecycle = { module = "androidx.lifecycle:lifecycle-runtime-compose", version.ref = "composeLifecycle" } turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" } composeStateEvents = { module = "com.github.leonard-palm:compose-state-events", version.ref = "composeStateEvents" } composeNavigation = { module = "androidx.navigation:navigation-compose", version.ref = "composeNavigation" } koin = { module = "io.insert-koin:koin-android", version.ref = "koin" } koin-compose = { module = "io.insert-koin:koin-androidx-compose", version.ref = "koin" } koin-test = { module = "io.insert-koin:koin-test-junit4", version.ref = "koin" } [plugins] androidApplication = { id = "com.android.application", version.ref = "gradlePlugin" } androidLibrary = { id = "com.android.library", version.ref = "gradlePlugin" } kotlinSerialization = { id = "org.jetbrains.kotlin.plugin.serialization", version.ref = "kotlin" } jetbrainsKotlinAndroid = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" } googleServices = { id = "com.google.gms.google-services", version.ref = "googleServices" } firebaseCrashlyticsPlugin = { id = "com.google.firebase.crashlytics", version.ref = "crashlyticsPlugin" } compose-compiler = { id = "org.jetbrains.kotlin.plugin.compose", version.ref = "kotlin" }" /> | ||
| </PendingDiffInfo> | ||
| </value> | ||
| </entry> | ||
| </map> | ||
| </option> | ||
| </component> | ||
| </project> No newline at end of file |
There was a problem hiding this comment.
The .idea/copilotDiffState.xml file should not be included in version control. This is an IDE-specific configuration file that contains pending diff information and is typically user-specific. It should be added to .gitignore to prevent it from being tracked.
|
|
||
| internal fun EntryProviderScope<NavKey>.homeEntry(navigator: Navigator) { | ||
| entry<Destination.Home> { key -> | ||
| val viewModel: HomeViewModel = koinViewModel { parametersOf(key) } |
There was a problem hiding this comment.
The @Provided annotation is being used to mark the params parameter, but HomeViewModel does not accept a Destination.Home parameter and will fail when Koin tries to inject it with parametersOf(key). This inconsistency between HomeSecondViewModel (which has a params parameter) and HomeViewModel (which doesn't) needs to be resolved. Either remove the parametersOf(key) call in HomeEntry.kt for HomeViewModel, or add a @Provided params: Destination.Home parameter to HomeViewModel if the destination data is needed.
Suggested change
| val viewModel: HomeViewModel = koinViewModel { parametersOf(key) } | |
| val viewModel: HomeViewModel = koinViewModel() |
feature/template-194-navigation3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this important?
implements #194
Notes
I chose not to use the Koin-Navigation3 integration, because it only gives very small benefits in developer ergonomics, while you tightly integrate DI with nanigation (you have to use the Navigator provided by Koin, while we prefer to have that separately implemented)
todo