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

Remove the trailiing slash from a base_path'd router's root URL #2395

Closed
wants to merge 2 commits into from

Conversation

pyrrho
Copy link
Contributor

@pyrrho pyrrho commented May 7, 2024

Currently, given a Dioxus.toml with base_path = "base/path", navigating to e.g. localhost:8080/base/path will result in the router pushing /base/path/ to the web history API, and therefore rendering localhost:8080/base/path/ in the browser address bar. This PR attempts to remove the trailing slash from the root url of base_path'd routers without breaking the functionality of apps that do not specify a base_path.

This isn't a functional bug, but it's a peeve of mine, and I wanted to figure out why this was happening. Additionally, this behavior is inconsistent with

  • axum routers that are hosting dioxus router apps; calling .nest("/base/path", MyDioxusMethodRouter) will result in browsers rendering e.g. localhost:8080/base/path/ as the root of the dioxus app, but might cause direct navigations to localhost:8080/base/path/ to 404. (NB. this is probably an issue with nested fallbacks in axum, but it was very frustrating for me while I was figuring that out.)
  • subroutes in the same dioxus router; adding e.g. #[route("/not/home/")] (note the trailing slash) will let both localhost:8080/base/path/not/home (note the not trailing slash) and localhost:8080/base/path/not/home/ to resolve to localhost:8080/base/path/not/home (again, note the lack of trailing slash).

I'm opening this as a draft because I am not at all confident in the implementation. The fundamental change is displaying the root URL of routers as "" rather than "/". This might be a bad idea. As you can see from the modified tests, this change requires special-case'ing Links that direct to the root url for non-base_path'd apps. Seeing href="" in an <a> is... kinda weird. Maybe a bit of a red flag. WebHistory::full_path does correctly resolve that to "/" before pushing to the history API, but I've not tested this behavior with liveview or native apps, nor have I tested this change with routers that use the #[layout(...)] or #[nest(...)] features.

I'm planning on building some axum experiments with this change in place, so I may update the PR as I find issues. If there's no additional movement on this PR from myself or the dioxus team in, idk, a few weeks I'll probably close this PR to keep it from taking up space.

@jkelleyrtp
Copy link
Member

jkelleyrtp commented Jul 31, 2024

Closing in favor of #2756 but thank you for the PR!

We will borrow some of the research here in the PR and add you as an author.

@jkelleyrtp jkelleyrtp closed this Jul 31, 2024
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