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

Unexpected Behaviour in 0.17.0-beta.1 #860

Open
No9 opened this issue Dec 12, 2021 · 5 comments
Open

Unexpected Behaviour in 0.17.0-beta.1 #860

No9 opened this issue Dec 12, 2021 · 5 comments

Comments

@No9
Copy link

No9 commented Dec 12, 2021

Hey All - long time no see.

Using 0.17.0-beta.1 - I have a static file server at

app.at("/*").serve_dir(path)?; 

And the following additional route defined.

app.at("/:slug/").get(somecall);

When I request http://localhost:8080/index.html I am routed to the app.at(/:slug/).get(somecall); route.

However as the path doesn't contain a trailing slash I would expect it to hit the serve_dir route.

Looking at the tests in https://github.com/jbr/routefinder/blob/main/tests/tests.rs I can't see a way to achieve what I am looking for.

Is the expected behavior?

Edit:
In 0.16 I was using app.at("/").serve_dir(path)?; without the * and it was working as expected.

@jbr
Copy link
Member

jbr commented Dec 13, 2021

This is—at least for me—expected behavior. Star routes are the lowest precedence in routefinder. Routefinder also ignores trailing slashes. I haven't thought a lot about how this works with tide in a bunch of months, but it seems like the way I'd describe this behavior verbally would be "if there's a static file at a given request path, serve it. if not, try matching the single-param slug route." This seems to me like the serve-dir behavior should be mounted as a middleware in tide terminology, in order to be able to pass through to the :slug endpoint.

Alternatively, I can look into making trailing slashes optionally semantic, but when I was designing routefinder I came to the design perspective that it's far less frequently desired to route a trailing slash distinctly from the otherwise identical path with the trailing slash stripped. I still believe this is the right behavior for most users (and is what I'd want for trillium, which shares routefinder)

@jbr
Copy link
Member

jbr commented Dec 13, 2021

Also — I wouldn't have guessed that it's necessary to mount the serve_dir at "/*". It should work to mount it at "/", I would have expected. But I haven't looked at tide in a while

@No9
Copy link
Author

No9 commented Dec 13, 2021

Thanks @jbr
The trailing slash convention comes from the static site generator (SSG) projects that I am integrating with.
e.g. gatsby 11ty etc
If found it a little odd at first too but it makes sense when dealing with content systems rather that APIs.
i.e. a content item maybe more than one file deployed to an endpoint.
If routefinder/tide/trillium are interested in supporting SSG folks it might be a handy feature to toggle?

And thanks for the idea of implementing the file serving as middleware it gives me a bailout option.

@mdtusz
Copy link

mdtusz commented Jan 21, 2022

In many other servers (e.g. express), the router has a strict configuration option - it may be useful to add this for tide routing as well as an option on Server rather than on an individual Route.

let app = tide::new().strict_routing(true);

or

app.at("/foo/:bar").strict(false).get(some_handler);

(or both?)

I don't love that as an API though. @jbr do you have thoughts on a good pattern or method of configuration for the Server? It's very likely that as usage of tide increases, more use-cases for configuration will arise. Tacking on setter-methods feels somewhat icky, but a ServerConfig struct or something of the like may be a bit heavy-handed.

@jbr
Copy link
Member

jbr commented Jan 21, 2022

This is a @yoshuawuyts question

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

No branches or pull requests

3 participants