AlexGalays / abyssa-js

Framework agnostic hierarchical router for single page applications
MIT License
71 stars 9 forks source link

Unknow query #9

Closed pauldijou closed 10 years ago

pauldijou commented 11 years ago

Hey,

So there is a way to specify dynamic params in the url and in the query string, like:

/contact/:who/:how?subject&from

Next, if I understand correctly, inside enterPrereqs and enter methods, you will have a params object looking like:

{
  who: ...
  how: ...
  subject: ...
  from: ...
}

So far, so good. But what if my url is:

/contact/you/mail?subject=Hey&from=me&unknow=trololo

Is there a way to access the unkown key of the query string? Even worse, what if my url is:

/contact/you/mail?subject=Hey&from=me&who=trololo&how=busted

Maybe having only one object is nice but this might create some naming conflict...

AlexGalays commented 11 years ago

Yes, unknown is accessible. Declaring a query param in your route means you own it (this is explained in the doc; it will change the state machine behavior), but you can access ANY query param from ANY state.

As for the last example, I think it's a bit contrived. You have full control over your path/query names so there is no reason you have to write it like that. At the end of the day, an URL represent one global state; it should be easy to find different names for the different substates that compose it. It's a tradeoff: By making the signature of the enter functions twice simpler, you increase the risk of collision by 2, but that risk should still be very low :)

pauldijou commented 11 years ago

By making the enter functions twice simpler, you don't increase the risk, you create one where there could be none. The code is at the mercy of any user since they can pass any query they want with any name they want. Writing an url like:

/contact/:to?subject

is totally legit but there is no way to prevent a user from calling:

/contact/you?subject=trololo&to=hackingYouRightNow

And your params are potentially all messed up. I know JavaScript isn't typesafe and all, but still, mixing url params and query params seems more like a source of problems that I would be totally fine to avoid by having one more argument in entry of... enter.

Another solution would be to ensure that url params override any query params with the same name. That would mean losing some information, but since you didn't really expect it, it might be fine.

sompylasar commented 11 years ago

Maybe one could prefer to have the params separated but have them in a single object to pass around, e.g.

var params = {
    path: {
        to: "you"
    },
    query: {
        subject: "trololo",
        to: "hackingYouRightNow"
    }
};

Although that's not a question of hacking. You might access the following URL with the same result:

/contact/hackingYouRightNow?subject=trololo
AlexGalays commented 11 years ago

Yeah, "hacking" the url is not really hacking :)

I'm not sure the params with two inner objects is an improvement. The current approach is simple and unless I miss some use cases, work just fine?

sompylasar commented 11 years ago

Yes, the current approach works. As soon as one defines both path param names and query param names, one should name the path params different from the known query param names. Hope I'm not missing something important.

AlexGalays commented 10 years ago

Ok, I can understand why some people may favor the separation of path and query params; But I fail to see how it's a definite improvement, considering the current approach is simple and works fine. I'm closing the issue, but happy to reopen it if limitations linked to the current implementation are discovered.