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

Fixed asset path resolver to use the second segment instead of first. #3472

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

sjud
Copy link
Contributor

@sjud sjud commented Dec 30, 2024

In the dynamic path example

fn app() -> Element {
    use_asset_handler("logos", |request, response| {
        // We get the original path - make sure you handle that!
        if request.uri().path() != "/logos/logo.png" {
            return;
        }

        response.respond(Response::new(include_bytes!("./assets/logo.png").to_vec()));
    });

    rsx! {
        document::Link { rel: "stylesheet", href: STYLE }
        h1 { "Dynamic Assets" }
        img { src: "/logos/logo.png" }
    }
}

The expected behavior is that because /logos/logo.png has first path segment "logos" that it would match the use asset handler with the name argument of "logos"
except this code

 // todo: we want to move the custom assets onto a different protocol or something
    if let Some(name) = request.uri().path().split('/').next() {
        if asset_handlers.has_handler(name) {
            let _name = name.to_string();
            return asset_handlers.handle_request(&_name, request, responder);
        }
    }

Doesn't do that.
Given a uri path of "/logos/logo.png" the first element of the array of the split method is the segment before the first '/' thus "" which is not the first path segment but the first skip result.
The expected behavior is that this would be the second element of the skip result.
I added .nth(1) to satisfy this.

@jkelleyrtp
Copy link
Member

Good catch, thank you!

@jkelleyrtp jkelleyrtp merged commit 0484106 into DioxusLabs:main Jan 2, 2025
15 of 17 checks passed
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