IssueTracker

since it is already marked as deprecated, we can probably do it by now.

Thank you for your response.

Serializing null as "null" makes null indistinguishable from "null". Of course it works when entering from a deep link.

• https://example.com --> param is null
• https://example.com?param=null --> param is "null"

But it doesn't work as expected when using internal navigation, i.e. navigate(NotHome()).

• NotHome(param = null) --> param is "null" (exptected: null)
• NotHome(param = "null") --> param is "null"

Probably this is the limitation of Navigation component that serializeAsValue is always called (no option to omit an argument from being serialized)?

Even forcing the property to be not-nullable also fails when the deep link is https://example.com?param=null (java.lang.IllegalStateException: Unexpected null value for non-nullable argument param).

By the way I want to highlight this. This causes us not to be able to use String property, because anytime there can be "null" querystring that will make the app crash.

It is less about serializeAsValue and more about the built-in NavType.StringType adhering to kotlin's null.toString convention.

Though I'm curious why you can't do this

val param = bse.toRoute<NotHome>().param ?: "null"
Text("param=$param")

By the way I want to highlight this. This causes us not to be able to use String property, because anytime there can be "null" querystring that will make the app crash.

Yes, when you explicitly declare a non nullable String type, a null value is not allowed. That is part of type-safety. And to ensure deeplinks are type-safe, you should add deeplinks via the new type-safe deepLink Builder

composable<NotHome>(
    deepLinks = listOf(navDeepLink<NotHome>(basePath = "example.com") { ... }),
) { bse -> 
    ...
}

Though I'm curious why you can't do this

It still makes "null" indistinguishable from null. This is not ideal from an idealist's point of view :D. However, from practical point of view, if param is a search query or an initial value of a TextField, "null" within a deep link does mean "null" while the absence (null) or "" both mean "", it will never be null, so it is String instead of String?. This is not an obstacle, since we can make a custom NavType for String that is consistent and symmetric, it is just inconvenient because we need to do this manually by passing a custom NavType to every destination declaration to avoid crash.

This is just my opinion when evaluating the beta version. In the end, the decision is yours.

to ensure deeplinks are type-safe, you should add deeplinks via the new type-safe deepLink Builder

Either the deep link is declared with full placeholders or with navDeepLink<NotHome>(basePath = "example.com"), in case NotHome.param is String (not nullable) and is optional:

• Deep links from outside (which we can't control) such as https://example.com?param=null will make the app crash instead of fall back to the default destination (start destination of the root graph).
• Internal navigation such as navigate(NotHome("null")) will also throw an exception, thus "null" is considered an invalid argument that needs to be checked prior to each call to navigate.
  • This is the same problem as navigate(NotHome("")) if param is a required argument, because in this case the (filled) route is android-app://androidx.navigation/com.example.nav.NotHome/ which doesn't match the pattern android-app://androidx.navigation/com.example.nav.NotHome/{param}. This also needs to be checked prior to each call to navigate. Otherwise, an exception might be thrown. (EDIT: Fixed in 2.8.0-beta04)

In the end, to be safe, a string argument must always be String?, or String with custom NavType.

Regards,

null means absence of value, while "" is a value of empty string - they certainly do not both mean "".

I'm getting some contradictory messages here - you want to support deeplinks that pass in null (which may or may not be "null" given that its a string deepLink), yet you want the param to be String instead of String?. You seem to be saying we will support deeplinks passing in null, but we also don't.

Granted that kotlin's null.toString() returns "null", how do you know for sure that the source of the deeplink, which you don't control, didn't pass in null instead of "null"? In that case your use of String would be incorrect.

The mix use of "null" with non-nullable types also kind of circumvents Kotlin's effort to ensure null safety. My opinion is that it should be avoided.

I did change my mind over time. The "last state of my mind" about String and String? is:

1. If the type is String:
   Expected: It should never be null.
   Actual: String "null" in the URI (deep link) is treated as null so that runtime error occurs, because String type is not nullable.

2. If the type is String?: I don't think String? make sense, because a string parameter cannot be expressed as null in a URI (deep link), except by omitting both the key/name and the value (i.e. absence of parameter). However, absence of parameter in Navigation component means fallback to the default value. Meanwhile, default values can be anything, they are not always null. So, I have no strong opinion about this.

Conclusion: My primary concern is about number 1, because it can make apps crash.

Similar to the work done in b/351994237 to support nullable Ints (something that the default IntType does not support), perhaps there's a similar improvement we can do here where we should only use the default StringType for String? types (where "null" would still be parsed as null) and instead have a separate, internal type specifically for usage of String in a Serializable class that does not support null and won't ever parse any string, including "null" as null.

That would prevent any crashing and ensure that developers are in full control of whether they want String? or String in their Serializable type and ensure that no matter which option they choose, their app would not crash.

Fixed internally and will be available in navigation 2.8.1

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.navigation:navigation-runtime:2.8.1
• androidx.navigation:navigation-testing:2.8.1

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.navigation:navigation-common:2.9.0-alpha01