Skip to content

Conversation

@nazaburo
Copy link

@nazaburo nazaburo commented Oct 19, 2025

Fix sharedKoinViewModel fails with type-safe navigation routes #2292

@nazaburo nazaburo changed the title Fix issue (#2292) Fix sharedKoinViewModel fails with type-safe navigation routes Oct 20, 2025
@arnaudgiuliani
Copy link
Member

I don't see why it bring stronger verification here, as the compiler should already help here.
I believe more in a K 2.2 compat problem.

@arnaudgiuliani arnaudgiuliani added this to the 4.2.0 milestone Oct 20, 2025
@nazaburo
Copy link
Author

@arnaudgiuliani
Thanks! That makes sense.
However, I think this behavior comes from the fact that navGraphRoute is declared as Any?.
In that case, the compiler won’t treat it as a String, so the String function overload isn’t resolved without explicit handling.
To me, this looks like expected Kotlin behavior rather than a K2.2 compatibility issue.

@arnaudgiuliani
Copy link
Member

Do you have a test case to reproduce, something we can add into examples project, the android compose app?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where sharedKoinViewModel fails when using type-safe navigation routes. The fix adds conditional handling to support both string-based and type-safe (object-based) navigation routes.

  • Adds type checking to handle both String and non-String route types
  • Ensures getBackStackEntry is called appropriately for both route formats

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +87 to +91
if (navGraphRoute is String) {
navController.getBackStackEntry(navGraphRoute)
} else {
navController.getBackStackEntry(navGraphRoute)
}
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both branches of the if-else statement execute identical code. The conditional check serves no purpose and should be removed, leaving only navController.getBackStackEntry(navGraphRoute).

Suggested change
if (navGraphRoute is String) {
navController.getBackStackEntry(navGraphRoute)
} else {
navController.getBackStackEntry(navGraphRoute)
}
navController.getBackStackEntry(navGraphRoute)

Copilot uses AI. Check for mistakes.
@nazaburo
Copy link
Author

@arnaudgiuliani
I've added test cases to sample-android-compose demonstrating the fix.

Added:

  • Type-Safe navigation routes (NavigationGraphRoute, FirstScreenRoute, SecondScreenRoute)
  • Two navigation examples:
    • Type-Safe Navigation (navigationTypeSafeGraph) - Uses @Serializable route objects
    • String-based Navigation (navigationStringGraph) - Uses traditional string routes
  • Two screens sharing a SharedViewModel via sharedKoinViewModel
  • Simple ViewModel to verify the same instance is shared between screens

You can test it by running the app and tapping "(>)Type Safe Navigation Example" or "(>)String Navigation Example". Both examples demonstrate that screens share the same ViewModel
instance (shown by matching instance IDs and shared timestamp), confirming the fix works correctly with Type-Safe routes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants