Url.parse strips ? from :_id? type route params

[zimme]

Url.parse('/route/:_id/:slug?') don't handle the '?' part well,

it strips the '?' part and handle everything after it as query string or something.

Url.parse('/route/:_id?/:slug?') returns {..., pathname: '/route/:_id', path: '/route/:_id?/:slug?',...}
and pathname is used by normalize and other stuff that breaks optional params routing in iron:router.

I took a look at the code but I lack the in-depth knowledge to actually try and make a PR that might work with this and not skrew up the support for the old school hash frag support. Maybe I'm overthinking this but you know...

edit: Mixed up pathname and path, it's fixed now. The information is correct now

[zimme]

Also, related to this issue iron-meteor/iron-router#912

[tmeasday]

The issue is that various points (route init, middleware handler init) call Url.normalize() on the the path (which relativizes and removes search strings and query fragments). As @zimme pointed out, it does this by calling Url.parse on the path.

I don't think it makes sense to be "parsing" a parameterized path like that. There's a short circuit in there for regexp paths but that just seems like a bit of a hack. I would suggest we shouldn't be doing this, or we should just be relativizing the path, not trying to do more than that.

[cmather]

Not sure what the issue is here? Can someone explain? Url.parse is intended to parse a valid url into its component parts, including things like host, scheme, pathname, query, hash, etc. The url in the example above is meant to be compiled into a dynamic url. So in a url that needs to be compiled, the question mark indicates an optional dynamic parameter. But in a url the question mark indicates the beginning of the query parameters. So as far as I can tell Url.parse is don't what it should be doing. If you want to compile the url you can create a new Url instance and pass the url to compile as a parameter.

Tom, I think you're talking about something different here? But not sure it's related.

[tmeasday]

Possibly I mis-understood what @zimme was talking about but I was referring to the other issue where Url.parse() was being called on paths when it shouldn't have been. Perhaps irrelevant as you said @cmather

[zimme]

We could probably close this as this was an issue that I opened becase of iron-meteor/iron-router#912 and that one was resolved by not using Url.normalize in some place in the iron:middleware-stack package