-
Notifications
You must be signed in to change notification settings - Fork 3k
RFC: Default URL generation for optional parameters #1501
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
Comments
cc: @niemyjski @RodolpheGohard @SimpleAsCouldBe @stereokai @marcghorayeb @kevinrenskers @jshado1 @amcdnl @eranimo @wlingke @yohairosen @Siyfion @bobmash @crisbeto. |
Option B seems better for me, since I'm worried that pretty much all nested states where the parent had a default parameter defined would be, by definition, ambiguous. For example, is the URL |
@NevilleS that can be worked around in some cases using a regexp on the default parameter, which ensures the next segment doesn't match against the parameter. For instance, if :userid is always an integer value, then you could do |
I'd say those are exceptions rather than the rule, so the default behaviour would be to assume that squashing the slashes is unsafe, no? Otherwise this breaks the refresh behaviour for the majority of existing apps, I'd think... |
I'd say squashing is a bad idea. What's the point? It seems like it'd add complexity. |
It's nice that |
By the way, I can't cite the RFC or anything, but I'm pretty sure most pages treat multiple slashes as irrelevant. In other words, |
I think I like A over B since urls would look prettier - I tend to prefer prettier urls, and anything to help towards that end would be great. |
@wesleycho of course, you can still get squashed parameters with either scenario. With B you would have to add |
I added
|
I think from a use standpoint not squashing by default will lead to less confusions. While useful, this seems like it should be an opt-in feature. |
The potential ambiguity seems scary and bothersome. Couldn't the situation be detected and avoided? Consider this option D.
What if a special character, i.e. E.g. assume with two routes,
I think this accomplishes the goal of creating pretty routes while remaining easy to understand and predict. It would allows multiple routes to resolve to the same state, with the same parameters, but I can't think of a simple case where you would be routed somewhere unexpected. Is there a chance anyone is the wild is using a '-' as a path segment for anything meaningful? |
Yes 😄. URIs aren't a new thing (original RFC published in '98) and they mean a lot. For example, you can create a folder via Given that handing the URL is part of what browsers do, personally I wouldn't want ui router to do anything fancy about squashing parameters, substituting special characters, etc., since it's unclear how the behaviour would vary between Chrome, FF, IE, etc., and I'm inclined to be defensive when it comes to this kind of thing (here be dragons). But that doesn't mean I'm going to try to stop you cowboys from redefining what a URL means, by all means go for it! ... just not in the defaults 😄 |
It's also clear about what a fragment identifier is. Specifically,
It also contains gems like
and even
According to the RFC, i think everything being discussed is fair game. |
@wbk that's an interesting idea. I actually think that is how I would prefer my apps to squash urls if I were to squash them. It would preclude you from setting a parameter value to the special character "-" or "_" however. |
@wbk totally fair game, agreed. Although keep in mind ui router supports HTML5 mode which removes the |
Having A as the "out of the box" configuration would be inviting disaster. It would lead to a ton of "why doesn't my routing work" questions by people who aren't aware of it. "Best practices" and "tutorials" will quickly start suggesting replacing A with C until "you know what you're doing". On the other spectrum, the obviously least-surprising option is C. I think, if we are to only consider A, B or C, the point of contention here would be whether B is worth placing over C. While I can see the merit behind B, I can envision an outcry of "Double slash urls are ugly!" against it - it's just not something commonly seen on the web, currently. I think you should have C as the out-of-the-box configuration - as the least hassle when supporting ui-router. |
I would be alright with having A available optionally, though most likely I would stick with C. |
Hey, sorry i'm late to the discussion! |
I concur with everyone? above: C C is the most obvious, and virtually no WTFs/min. B looks weird to me, like something broke. I would rather something look a little confusing than for it to look broken. Probably a few WTFs for someone who didn't read the documentation too well. A is more for the advanced user: The potential to "break" and high(er) rates of WTFs/min, so I think it should be an opt-in. I'm definitely looking forward to A, but I think C should be the default.
Just saw in the comparison that you already thought of the boolean thing. |
I gave my input without some of these other choices around - I think C should be the default, it does not affect current users. A is a nice feature, but as others have said, if the choice of it being opt-in exists & isn't onerous, it is preferred to avoid breaking changes. |
Totally agree. Option C makes the most sense as the default value. |
Thanks all for your comments. I have one more thing to note, considering option C (nosquash) seems to be favored. With our sample states:
If a user manually types a URL like: |
@christopherthielen you're saying that if user goes to Or is it that |
@christopherthielen If the username resolves to null, will it consider the route to be invalid, and thus try to validate the route against another state, or will it just halt there ? |
Sorry for chiming in so late, been quite busy. I don't want to answer in Chris's stead but I believe what he meant was that accessing a state with an optional parameter, without passing the parameter in the url, would result in falling back to the default parameter, rewriting the url suitably (i.e., no I hope that makes sense. In regards to the original question, having spent hours implementing my fork of UI-Router supporting optional parameters, I am well aware of all the difficulties, both in code and in user experience, the desired flexibilty of such a squashing model brings. However, I have a proposal for a fifth option - Instead of squashing the default states (which leads to ambiguity), we should take a direct approach to avoid ambivalent URLs. If the URL part of a parent-state
This makes it clear - the value after the next slash belongs to Call it hindsight routing. Call pseudo-rewrite. Call it a protection mechanism or what have you - my original implementation relied on the same principle (but in reverse) to break apart URLs to figure which state the user asked for. |
@stereokai has a really nice option E. Of course, code wise it might be not as easy implementation as no-squashing, but from all of the options I would choose E. |
@DovydasNavickas yeah. basically, this:
|
@christopherthielen not quite, "squash slashes" collapses URLs, my proposal replaces the parts which are supposed to be "squashed" with disambiguative (is that even a word?) information, respective to each state in the URL construction... |
@christopherthielen, I still think that @stereokai suggested option E is really really good balance between user and developer friendliness. Any reasons not to make it a default behavior? |
@stereokai
that: Given:
Given:
|
@christopherthielen Exactly so. |
Maybe I'm missing something, but how does this scheme remove the ambiguity
|
@NevilleS agreed. There might be a bunch of substates too, each with their own names that may conflict with the :username parameter. It's a somewhat reasonable choice for url generation, but doesn't remove the ambiguity. However, I find it odd because At this point, I'm convinced that option C is the best choice (least surprising, always routes correctly) for defaults and the developer can then choose to squash. The code I have committed to master currently defaults to "nosquash" and supports "value" and "slash". I can't endorse option E at this time because it (1) seems more confusing to me, not less, and (2) I believe it requires knowledge of all immediate child states to properly implement. |
@NevilleS got a good point. Although it would be awesome to have ambiguity resolving done under-the-hood, I also agree that option E is not a solution, at least not yet. |
I've got a plunk partially implemented with 0.2.12-dev showing default param behavior with "nosquash". http://plnkr.co/edit/VMPc8D7oUG0B1R3QuiCE?p=preview What I said earlier about "/user/" rewriting to "/user/christopherthielen" seems false at first glance, because a |
OK, sounds good, |
@christopherthielen I played a bit with your plunk: What I added are four buttons for routing:
Is it expected behavior for And if I go to a johndoe and then click on "Null username", on first click it loads default param value and on a second one, it does load actual null value. Not sure if I'm missing something, or it is some kind of issue. |
Yeah that doesn't look like proper behavior. Null should be considered a value, while undefined should be considered "not a value so use default". I'll try to figure out why it's doing this a little later. |
@DovydasNavickas The null case is as follows:
Then, when clicking the button again, the same process runs, but the URL doesn't doesn't change, so $locationChangeSuccess is never fired. I'm not sure any of this is a bad thing. null can be used meaningfully for some other custom Type (as long as it encodes null to a string somehow). It can be used as the default value for a param, in order to define the param as optional. But in this case, you simply don't want to transition using { username: null }. I updated my plunk with a fix to empty-string mapping. It's now mapped from an empty string (in the URL) to undefined, for optional and array parameters. Tell me your thoughts. |
@christopherthielen @NevilleS @DovydasNavickas I can see now I haven't thought that through enough. Thank you guys! However, I'd like to try give you an additional point of view for analysis of my proposed solution. Given the following configuration (notice I have added a "parameter name" to each state's URL): userstate: { name: 'user', url: '/user/:username' }
gallerystate: { name: 'gallery', url: '/gallery/:galleryid' }
photostate: { name: 'photo', url: '/photo/:photoid' } Loading Compare this when accessing the default gallery for the "default" user (logged in user): What gets squashes are the parameters, while the real URL parts remain to reflect the "full URL" (which is something we are trying to avoid). This is one way using which it can be achieved, however, it might break the original purpose. |
I think Option E adds complexity whilst trying too hard to protect the dev from himself/herself. I would argue that the original pattern is flawed—all along it should've been: User is the section, Gallery is the view, and photoid is what should initially be displayed onload. Option D is kind of growing on me. It's nice to see the placeholder: But I still think Option C should be the default. @christopherthielen rewriting the url is what I expected to happen ( |
Thanks for your feedback! |
OK I have a functional demo running: http://plnkr.co/edit/VMPc8D7oUG0B1R3QuiCE?p=preview |
I implemented option "D" and removed the explicit "value" option. Now, I also added some logic to perform pre-decode mapping. This allows us to specify the mapping behavior between the URL (String based) and $state.go (object based). The default map for optional parameters will replace the empty string |
@stereokai what you ended up with is the same as option A, "slash" @jshado1 the url is rewritten when using $state.transitionTo but is not rewritten upon a $locationChangeSuccess. This is due to the url-to-state synchronization mechanism. sync direction 1
sync direction 2
|
@christopherthielen If 0.2.12 is already released, is this planned for 0.2.13 or 1.0.0? |
@ilanbiala it's in there :) |
Ah okay, cool! Thanks for the great features. |
nice work @christopherthielen and such fast turn-around for option D! Thanks |
So… does that mean there's nothing to listen to? (if not, any chance for something like |
@jshado1 here is the code that, upon matching a url, transitions to the state that was matched. Note
You might try decorating transitionTo when pre-rendering and forcing |
@christopherthielen doing squashing in 0.2.15. |
Squashing URLs
With the addition of optional URL parameters, UI-Router can now generate friendlier URLs. For example, with the following states created with default parameter values:
This RFC hopes to determine the default url squashing behavior. When the user navigates to
user
($state.go('user');
), the full URL with parameter included would be#/user/christopherthielen
. However, we can squash this URL down to:#/user/
or even#/user
by omitting the parameter's default value.When the user navigates to
user.photos
, the full URL with parameters would be#/user/christopherthielen/gallery/0
, but we can squash this down to:#/user//gallery/
or even#/user/gallery/
which looks even nicer.Of course, we can transition to the same
user.photos
state with non-default parameters too. Then, we cannot omit the parameter value; the URL must reflect the non-default parameters. For example,$state.go('user.photos', { username: 'someotheruser' })
will generate a full URL#/users/someotheruser/gallery/0
or a squashed version:#/users/someotheruser/gallery/
.Squashing slashes
Omitting the slashes from the URL can potentially generate ambiguous URLs
In the above example, if we navigate as follows:
$state.go('user.photos.photo', { username: 'christopherthielen', galleryid: 0, photoid: 21872 })
, the full URL (no squashing) would be:#/user/christopherthielen/gallery/0/21872
, the squashed URL would be:/#user//gallery//21872
, and the URL with slashes squashed would be:#/user/gallery/21872
.Note that
#/user/gallery/21872
(slashes squashed) is ambiguous. Is21872
the galleryid or is it the photoid? It was intended to be photoid 21872, but if the user hit reload in their browser, they would instead return to galleryid 21872.Here is another example from #1487. It sets up a state with 3 parameters, each with defaults.
If we do
$state.go('test', { bazid: 10000 })
and squash slashes, then the url generated is#/10000
. After a reload, that url would load statetest
with param values of{ fooid: 10000, barid: 456, bazid: 789 }
.#3 Parameter Squashing options
All three parameter squash options (
"nosquash"
,"value"
,"slash"
) will likely be available for use in for 0.2.12. The code to process them is in a branch, pending merging to master. You can view the branch here: https://github.com/christopherthielen/ui-router/compare/angular-ui:master...master ...You will be able to set UI-Router's default squash policy using
$urlMatcherFactoryProvider.defaultSquashPolicy()
. You will be also able to set the squash policy on individual parameters.state({ name: 'foo', url: '/foo/:fooid', params: { fooid: { value: 12345, squash: "nosquash" } } });
Default behavior? - Finally, getting to the point!
What behavior would you expect from UI-Router "Out of the box"
With the preceding states, when an optional parameter's default values is used, should the out-of-the-box policy be to:
#/user/gallery/
, but the developer may not notice that they generate ambiguous URLs such as#/user/gallery/21872
.#/user//gallery//21872
but will generally map back to the state and parameters that they're generated for.#/user/christopherthielen/gallery/0/21872
. This is the most verbose.Which option for UI-Router would surprise you the least?
The text was updated successfully, but these errors were encountered: