Make trailing slash optional in splat params.

[DylanPiercey]

Hey,

I often find myself writing paths like this:

app.get('/test/*', ...)

// Which becomes

app.get('/test/(.*)', ...)

However 100% of the time what I actually want is this:

app.get('/test(/?.*)', ...)

Is it possible to make a splat only section of the path allow for optional trailing slash?

I realize this would be a breaking change, but I can't perceive any crazy issues with it and it would be pretty convenient for myself and perhaps others.

[blakeembrey]

You could do /:path* to allow multiple segments. You can also use a regexp if you need something more complex - E.g. /(.+)?.

[DylanPiercey]

I know there are ways to achieve this currently I just think the default isn't as useful as it could be.

I was also kind of surprised that

'/test/(.*)?'

Works perfectly but

'/test/*?'

Does not.

[blakeembrey]

It's documented in the README, and this is the reason I had disabled the * altogether. It was re-enabled because of the concern for people upgrading in the future from Express 4.x to 5.x. The asterisk doesn't make sense to /*? because you couldn't do /*+ or /**. The functionality mirrors path-to-regexp 0.x used in Express.js so I'm not sure there'd be any reason to change that.

[blakeembrey]

I'm going to close this since the behaviour is set and won't be changing. Even if it did change, there's no guarantee that instead most people really want the current functionality and you're just an outlier. I think the current behaviour is expected, the asterisk is not a param group - it's just an asterisk, but I will keep a look out for similar comments.

[blakeembrey]

Ok, so revisiting this from #92 (comment). If this change were to happen, it'd be kind of defeating the purpose I added it back to begin with (which was legacy Express.js support). Mostly likely, it'd be compatible with how most people use it - except for that small percent that may care about the prefix (for example, this module may be used in a different router that doesn't normalise requests like /foo and foo). What are your thoughts now?

Edit: Deleted content relating to how they match (E.g. /:path* won't match /foo////bar while /* does). That wasn't the purpose, got lost somewhere because splat makes me think of /:path*. I think thinking of * as a wildcard works, hard to see a conclusive reason to make it behave like params - anyone else can weigh in though, more opinions can't kill.

[DylanPiercey]

Heres my thinking -

Say you have a path /thing/:items* - It makes perfect sense to allow for just /thing since your saying 0 or more items using the asterisk. This is the way it currently works as well which is great. I'm not sure why /* is so special, IMO it should just be an unnamed group that functions the same, zero or more extra path segments.

Ultimately for me the /* intuitively says 'match anything and nothing' and since I am also saying trailing slashes are optional with strict=false then it should match without the trailing slash. Perhaps with strict=true the trailing slash could be required.

Although I think that this is the way it should work I totally understand where your coming from. It'd be nice to have others chime in but I personally don't think this change would effect many people.

Also if you are curious as to why I need this feature and created a wrapper, it is because I am using it here to facilitate mounting routers at paths. To me it makes sense to do it this way because you are explicitly saying at /api/* which is any path in the api vs /api like in express and allows for mounting in all of the routing methods. But that's just my particular use-case. The big problem here though is that when you mount at /api/* most people expect that it will also account for /api but instead it will only match /api/.

If you come to a conclusion on this please ping me.
Thanks again for taking the time to discuss this.

[blakeembrey]

Thanks for the link @DylanPiercey, that actually helps a lot. I don't have any conclusion still, but it looks like you aren't using { end: false } then? I'm leaning toward the opinion that mounting a router within a router like this appears natural, but is not correct. I think there's a few mounting use-cases that this would break because of the obscurity of use (does this have to match the full URL to continue routing instead of just the prefix?).

One big reason for mounting is to automatically strip the prefix. Does that library do this for you? Also, it looks like I should add some test-cases of the hostname routing 😄

[DylanPiercey]

Thanks for looking through the code. I am actually using end=false however I am doing some trickery to check that it did in fact match the whole path before running middleware. I did this because it allows me to pull off routing and mounting in one go by removing the part of the path that was matched.

Currently mounting in Rill doesn't update any existing path's/pathname's because I found that to be pretty invasive and haven't really needed it, however I just released a patch that exposes matchPath and matchHost which is effectively the remaining part of the path or host to be matched.

Could you elaborate on what test-cases I should add for hostname? That'd be extremely helpful.

Also, if your not super busy do you think you'd have a few minutes to discuss this further over irc, gitter or something? If not thats cool.

[blakeembrey]

Sorry, not you for the test-cases, me 😄 If you're using path-to-regexp for that, I should test it - there currently aren't any.

I can chat anytime, let me know where is easiest.

On stripping the prefix, it's actually extremely handy and something I regularly rely on, so you may want to consider support it. The big advantage of stripping it as you go down the stack is that you can mount third-party projects into yours without any code changes required. However, this might be less of an issue because you're doing something to cause the paths to keep matching as they go down anyway. But doing this ensures anyone using it, not just the router, can use it in any context easily. It's pretty cool to re-use someone else's router into yours (E.g. Kue comes to mind).

[DylanPiercey]

Ah okay, thought you were saying some tests were missing from Rill 😜.

I agree that mounting and faking that a nested app is at the top level is useful however in my experience i have found the big advantage comes from just the routing aspect of this and it's a bit of a leaky abstraction. It's even more difficult when you have phases to your routing which Koa and Rill do. If you look at koa-compose you can see how it has to juggle the path name back and forth which IMO is a little dirty, not to mention that it re-parses the path in koa for each of these switches. Also things like redirects just go completely wonky unless you patch them as well.

Because of that I just opted to storing a variable on the request which is the current unmatched part of the path (or hostname) and the routing middleware is set to use that instead of the actual path. I've found it works pretty good.

Thanks for taking the time to think about this further, much appreciated.

[blakeembrey]

Interesting, thanks for the links. I've built middleware with mounting in the past, and although it's not perfect it's not bad to add two lines before and after to strip and re-add it. The re-parsing is unfortunate, my first thought is that it could be improved though by making the get on this.url lazy instead of all the sets. You could also have the first get to this.url cache until future property changes to improve that situation, so I don't think it's worth worry about. Also, that's all internal details anyway.

The redirect issue is a good point too, but redirects can be relative so working around that is simple - just send relative paths instead of absolute and continue allowing absolute for the use-case it exists for. For DX, it might be worth adding two methods so people can avoid running into a case they didn't want. I don't think implicitly adding the mount path here is a good behaviour either, since it's very likely people want an absolute path in some cases - but I do agree it's a leak in terms of typical thinking, so you'd need to shape user expectations through the API instead.

On a side note, is that what we're calling that pattern - "phases"? I've been using it for HTTP requests also - see http://github.com/blakeembrey/throwback and https://github.com/blakeembrey/popsicle.

[DylanPiercey]

Yeah mounting has always been a tough one for me, although i'm pretty happy with the Rill router at the moment.

I borrowed the term phasesfrom the koa middleware guide, they mention the capture and bubble phase, so I figured together they were phases? Terminology for this kind of middleware is as hard to describe as it is to wrap your head around when you first see it 😜.

Also popsicle looks sweet, thanks for the link! Universal javascript libraries always make me happy.

If you have any more questions for me about how I am using path-to-regexp or whatever you can also feel free to hit me up on gitter - you'll get the fastest response there. I know I said i'd like to chat earlier but I think I got out what I needed to here 😀.

[DylanPiercey]

@blakeembrey - Ping me if this gets added or if it needs more discussion, appreciate you looking into it again.

[blakeembrey]

Absolutely, no worries. I was considering if I should just make standalone behaviours of ? and + which would allow * to act consistently with what you want. It just doesn't seem terribly useful, since it'd basically just map to .*, .? and .+, and the current behaviour would be gone. You'd end up being able to do things like mail.+.com or something. Does it make sense that + caters for the prefixed . while * doesn't?

[DylanPiercey]

I think make.+.com makes sense, basically allowing this kind of stuff just allows for unnamed params in routers. I personally think that this should compile to the same as make.:part+.com except make the name the current index of the capture group (or even just undefined).

Ideally all of these would be equivalent except without specifying names for the capture groups.

'make:part+.com'
'make.+.com'

'make:part*.com'
'make.*.com'

'make:part?.com'
'make.?.com'

To me that's how I intuitively think it should work, this way '*', '+', and '?' have the same functionality no matter where it is.

[blakeembrey]

Ok, I'm semi-onboard. If it acts the same, you lose the behaviour around "wildcard" matching everything like foo..bar or x//y//z because it gets based on delimiters instead. It's not a bad trade-off, it just means if people want to literally match everything they'll need to do (.*) from now on. One curious thing though, is whether people expect /:path+ today to match /foo//bar or not. Would you?

Edit: I feel like you would, since you're doing sub-routing. If it wasn't matched, things in the sub-route might be skipped that would otherwise be valid.

[DylanPiercey]

I personally wouldn't have expected /foo//bar to match, although it kinda makes sense. The output for path being [undefined, 'bar'] or perhaps ['', 'bar']?

I personally think it should be on the developer to properly format the url to /foo/bar in this case but I don't think users would be shocked if it matched or not, it's a weird url.

[DylanPiercey]

Ping @blakeembrey.

Do you think it's worth reopening this issue an/or perhaps getting some outside input somehow?

[blakeembrey]

Sure. Let me know how you'd like to get that 😄

[blakeembrey]

Let me know if this is still an issue. I'm dropping /* in 2.0 (again) because it's been a real pain to decide how it acts and encodes and everything else. I'll add it back if there's popular demand, but right now it made things like compile more complicated by having a special encodeAsterisk function that just skipped /'s. I'm not sure that's even accurate behaviour there, since I agree it should really act like (.*). I also added some more useful options that I thought you could use (specifically, delimiters and endsWith - might make host matching better). See #114.

Edit: And I've lost a lot of context on the next action item for this issue.

[DylanPiercey]

Thanks for the update. Going to dig into this new version now!

[blakeembrey]

Let me know if anything is missing 😄 I think it was mostly your use-case ideas that ended up with the new options.

[HansBrende]

@blakeembrey nearly the same issue over in URLPattern: whatwg/urlpattern#163
I would love your feedback on this, as I believe that this behavior is one of the two only major failings of URLPattern currently... and since all developers are going to be stuck with this behavior... forever... it seems important to implement correctly the first time! Before it is taken out of experimental status and goes more mainstream in more browsers.