Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support parsing navigation path arguments #1277

Merged
merged 2 commits into from Apr 17, 2024
Merged

Conversation

MatkovIvan
Copy link
Member

@MatkovIvan MatkovIvan commented Apr 16, 2024

Proposed Changes

  • Adopt basic deep-link matching
  • Add parsing parameters from "path" (without support of parameters in URI's query or fragment parts)

Testing

composable(route = "user/{id}") {
    println("id = " + it.arguments?.getString("id"))
}
navController.navigate("user/1")

Issues Fixed

Fixes MatkovIvan/nav_cupcake#4

@MatkovIvan MatkovIvan requested a review from igordmn April 16, 2024 21:25
@kropp kropp self-requested a review April 17, 2024 09:37
@MatkovIvan MatkovIvan merged commit cc346a0 into jb-main Apr 17, 2024
6 checks passed
@MatkovIvan MatkovIvan deleted the ivan.matkov/fix-nav-args branch April 17, 2024 09:38
@MatkovIvan MatkovIvan changed the title Support parsing navigation arguments Support parsing navigation path arguments Apr 17, 2024
MatkovIvan added a commit that referenced this pull request Apr 18, 2024
From
[docs](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.text/-regex/):
> For JS
>
> Note that RegExp objects under the hood are constructed with [the "u"
flag](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/unicode)
that enables Unicode-related features in regular expressions. This also
makes the pattern syntax more strict, for example, prohibiting
unnecessary escape sequences.

JS has stricter regex validation rules, so copied from android regexes
doesn't work.

```
Invalid regular expression: /(\?|\#|$)/gu: Invalid escape
SyntaxError: Invalid regular expression: /(\?|\#|$)/gu: Invalid escape
SyntaxError: Invalid regular expression: /^multiplatform-app://androidx\.navigation/ConfigurablePopup($|(\?(.)*)|(\#(.)*))/giu: Invalid escape
```

IDEA also thinks that it's "redundant"

<img width="524" alt="image"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/1836384/ca91e003-4a77-4777-9287-ba4a32806943">


## Testing

Test: run mpp on JS target

## Issues Fixed

Fixes regression after
#1277
@VishalGopalrao
Copy link

java.lang.IllegalArgumentException: Navigation destination that matches request NavDeepLinkRequest{ uri=android-app://androidx.navigation/Settings } cannot be found in the navigation graph ComposeNavGraph(0x0) startDestination={Destination(0x78c9ba0c) route=Home}
at androidx.navigation.NavController.navigate(NavController.kt:1822)
at androidx.navigation.NavController.navigate(NavController.kt:2234)
at androidx.navigation.NavController.navigate$default(NavController.kt:2229)
at androidx.navigation.NavController.navigate(NavController.kt:2211)

@MatkovIvan
Copy link
Member Author

@VishalGopalrao it says almost nothing.

  1. If it's android (uri=android-app://...), please open the issue in Google tracker - Multiplatform version uses original Google's binaries on Android
  2. If not, please open a separate issue with full description, version and reproduction steps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants