Nullable Navigation arguments are incorrectly marked as "required" and app crashes. [198689811]

Fixed

Bug

Status Update

No update yet.

Description

br...@gmail.com

created issue #1

Sep 3, 2021 01:11PM

Component used: Navigation Version used: 2.4.0-alpha08 Devices/Android versions reproduced on: All

It seems that the fix for https://issuetracker.google.com/issues/185527157 has introduced another bug. Nullable arguments with an @null default value are incorrectly treated as required, and cause the app to crash if they are not present in a deep link. You will get the following error:

java.lang.IllegalArgumentException: Deep link myurl.com/program/{id}?someArg={someArg} can't be used to open destination Destination(com.someapp.android:id/SomeFragment) label=SomeFragment class=com.someapp.android.features.SomeFragment.
    Following required arguments are missing: [someArg]

The Nav Graph defines the argument like this:

<argument
    android:name="someArg"
    app:argType="string"
    app:nullable="true"
    android:defaultValue="@null"/>

Comments

jb...@google.com <jb...@google.com> Sep 3, 2021 03:33PM

Assigned to sa...@google.com.

ap...@google.com <ap...@google.com> #2Sep 7, 2021 10:34PM

Project: platform/frameworks/support
Branch: androidx-main

commit e1b42da3ee5bcb06756d8cb8377abf6116086e11
Author: Sanura N'Jaka <sanura@google.com>
Date: Tue Sep 07 20:23:52 2021

Make nullable NavDeepLink arguments not required

Removing the inclusion of nullable NavDeepLink
arguments from the "required" argument checking.

RelNote: "Nullable `NavDeepLink` arguments no longer
require a default value."
Test: deepLinkNullableArgumentNotRequired test
Bug: 198689811

Change-Id: Ia14ef04bfe5b25942163688f40adacc30fa7e044

M navigation/navigation-common/src/androidTest/java/androidx/navigation/AddInDefaultArgsTest.kt
M navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M navigation/navigation-common/src/androidTest/java/androidx/navigation/test/NavArgument.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.kt

https://android-review.googlesource.com/1817360

il...@google.com <il...@google.com> #3Sep 8, 2021 06:16PM

Marked as fixed.

There's two issues here:

android:defaultValue="@null" is actually treated as the defaultValue attribute not existing at all at runtime (we do still use it for Safe Args)
We enforced every argument without a default value to be present in the URI/route, even if it was nullable.

While we can't fix the first one, we have made the change in Navigation 2.4.0-alpha09 to only check for default values for non-nullable parameters. This should mean that

Using android:defaultValue="@null" won't be a problem
Constructing a type in the Kotlin DSL via navArgument("argument_name") and making it nullable = true without a default value won't be a problem.

sj...@gmail.com <sj...@gmail.com> #4Sep 17, 2021 10:46AM

Unfortunately I still experience this bug with 2.4.0-alpha09 :(

java.lang.IllegalArgumentException: Deep link penny://app/offers can't be used to open destination Destination(de.penny.app.debug:id/offerListFragment) label=OfferListFragment class=de.penny.feature.offer.view.OfferListFragment.
    Following required arguments are missing: [sectionIndexOrName, raffleId]

This is the excerpt from my navigation graph XML:

<argument
    android:name="sectionIndexOrName"
    android:defaultValue="@null"
    app:argType="string"
    app:nullable="true" />

<argument
    android:name="raffleId"
    android:defaultValue="@null"
    app:argType="string"
    app:nullable="true" />

<deepLink
    android:id="@+id/offersDeeplink"
    app:uri="penny://app/offers" />

As you can see both parameters are nullable and have a default value of null.

il...@google.com <il...@google.com> #5Sep 20, 2021 12:00AM

Re comment#4 - please file a new bug with a sample project that reproduces your issue.

sj...@gmail.com <sj...@gmail.com> #6Sep 22, 2021 08:56AM

I'm sorry, I forgot to upgrade the Safe Args Gradle plugin as well. However when upgrading the plugin I now get another error.

Issue 198689811