Fix nesting Router at / leading to double slash in the path

[davidpdrsn]

If you nested a Router at / the path for the final routes would start with a double slash.

Fixes #690

Aside about how nesting works:

When you nest a service we try and downcast it to a Router. If that succeeds we simply move all the paths from one router onto another, with the path prefix added plus a middleware that strips the prefix before calling the MethodRouter. That simplifies lots of things internally because there is only one router that has all the paths. It also improves performance since matchit can see the entire tree. If the downcast fails we just nest it as an opaque service however, don't really have other options.

This is why nesting a Router at / is fine, and wont collide with all other routes, but nesting something like ServeDir will.

[jplatte]

So this will be supported while .nest("", /* router */) panics? That seems inconsistent to me.

[davidpdrsn]

We could make nesting at "" work. Its just a simple check currently. Should it work the same as /?

[jplatte]

Yeah I think it would make sense for both to behave the same way, whether it works the same as merge or is a panic (in which case the panic message should probably mention merge).

In terms of implementation, wouldn't it be simpler to just delegate nest("" | "/", _) to merge(_)?

[davidpdrsn]

Yeah I think it would make sense for both to behave the same way

Done!

In terms of implementation, wouldn't it be simpler to just delegate nest("" | "/", _) to merge(_)?

I thought so as well but the behavior regarding fallbacks is slightly different between the two. nest always panics if you nest a router with a fallback, because we merge all routes into a single a router and thus can only have one fallback. Supporting nested routers with fallbacks would be nice but required something new in matchit iirc. Whereas merge only panics if both routers have a fallback, because it wouldn't know which one to pick.

[jplatte]

Done!

You didn't push though 😅

[davidpdrsn]

whoops!

[Atrox]

Just as an info:
This change did break my functioning setup of nested routes. I needed to add a / prefix to all of them to work correctly with v0.4.4.

base:

    Router::new()
        .nest("/", pages::routes())

pages::routes before:

    Router::new()
        .route("/", get(index))
        .route("something/:id", get(something))
        .route("other", post(other))

pages::routes now:
(this did not work for me before v0.4.4)

    Router::new()
        .route("/", get(index))
        .route("/something/:id", get(something))
        .route("/other", post(other))