Skip to content
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

Make "/" and "" equal the same thing #182

Closed
timkindberg opened this issue Jun 14, 2013 · 18 comments
Closed

Make "/" and "" equal the same thing #182

timkindberg opened this issue Jun 14, 2013 · 18 comments
Assignees
Milestone

Comments

@timkindberg
Copy link
Contributor

Sounds like everyone is on board with this. Make the use of "/" equivalent to using "" (empty quotes). See #174 (comment)

@nateabele
Copy link
Contributor

👍

@ksperling
Copy link
Contributor

I suppose this should happen inside urlRouter. We currently simply pass the $location object into the matcher functions; maybe we need to change this to pass (path, $location), where path has pre-processing like "" -> "/" applied.

I don't really like how $urlRouter has been changed to using $injector.invoke either; I don't disagree that having callbacks injectable is a good idea, but I think there's a better way to do that rather than using $injector.invoke everywhere.

I'm looking at implementing $injector.bind(fn) to do something similar to Function.prototype.bind(), i.e. return a new function object where injectable parameters are already bound instead of having to be looked up on every call. Additionally you'd be able to call $injector.bind() during the config phase, and get a bound function returned immediately, but invoking the function will fail until runtime when the injectables have actually been bound.

@aaronleesmith
Copy link

Is this issue referring to the fact that URLs ending in '/' don't route correctly?

If not, I opened issue #205 to address this issue.

@ghost ghost assigned timkindberg Aug 7, 2013
@timkindberg
Copy link
Contributor Author

I was gonna mess around with this one, to see if I could fix it, but I'm finding that I can't even get the current method working. If you have a parent with url of "/parent" and a child with url of "" (empty quotes) is the child supposed to be activated along with the parent when navigated to "/parent" in address bar?

Plunkr showing the issue: http://plnkr.co/edit/9VBBiC?p=preview

@ksperling
Copy link
Contributor

@timkindberg having the child state URL be the same as the parent url (by appending '') really only works if the parent is abstract.

I think for the "" vs "/" thing the correct fix is to change UrMatcher to silently treat $location.path() == "/" as if it was "". (I think doing it this way around as opposed to treating '' as '/' is more symmetrical with how the default child of any other abstract parent also uses '')

@ksperling
Copy link
Contributor

In addition, compiling an (absolute) pattern of "/" into an URLMatchers should just silently return a UrlMatcher for "" (which will then match both "" and "/").

Actually that makes me think it should be the other way around... compiling an (absolute) "" UrlMatcher will give you "/" instead, which silently also matches "", but .format() will return "/" so that generated URLs are still correct.

@nelsonpecora
Copy link

I think there should be two config options:

  • "fudge urls" (or whatever it's called) - when this is on, things like "/" and "", "/foo" and "/FOO", and such will route the same
  • "trailing slash" - this is only applied if the first option is true. when it's on, non-trailing-slash routes get a trailing slash, and when it's off trailing slashes are removed

These two options would cover all of the functionality that vanilla ngRoute has in this regard, without forcing a certain URL style (for example, the app I'm building requires trailing slashes in the urls, and some people want to use case-sensitive routes).

@timkindberg
Copy link
Contributor Author

I'm fine with those two options if anyone wants to submit a PR.

@nelsonpecora
Copy link

I'm going to dive into the code this weekend and see if I can get this working.

EDIT: Fleshing out this idea, how does this sound?

$stateProvider.trailingSlash(); // true: add slash, false: remove slash (if you don't call this method, there's no trailing slash fudging)
$stateProvider.lowerCaseUrls(); // true: make urls lowercase
$stateProvider.upperCaseUrls(); // true: make urls uppercase (whichever one comes latest gets set, if none are called there is no case fudging)

@timkindberg
Copy link
Contributor Author

So the case insensitive stuff was just added: #957

Only the trailing slash would be needed now. And it would be part of UrlMatcher just like the PR linked above, since all url matching happens there. Or at least not part of $stateProvider.

tlvince added a commit to tlvince/LMIS-Chrome that referenced this issue Mar 17, 2014
`$scope.go()` inside the top-level run block will increase test complexity as a
mock template cache would need to be set up for each test that uses `$scope`
regardless of whether it depends on a template.

UI Router currently uses a state using an empty `url` as the default state, see:

angular-ui/ui-router#182
angular-ui/ui-router#174 (comment)

Since we're setting up the layout in a parent, abstract state, redirect to the
*real* index using `otherwise()`.
@moorthi07
Copy link

This will kill the HTML standard use of '#' navigation. Right now it is not working with #, every #identifier link is replaced to a #/identifier url. Can you guys stop messing html standards?!

@nateabele
Copy link
Contributor

I'm pretty sure this is resolved (and, correctly) by the $location service.

@moorthi07
Copy link

are you saying, use $location instead of url like below?
.state('contact', {
url: '#contact-div', // $location.path('#contact-div')

    templateUrl: ''

  })

@nateabele
Copy link
Contributor

I'm saying that, under the hood, UI-Router uses the $location service.

@moorthi07
Copy link

I know. Just to make it easy,
How would you do this simple page with ng-ui when I DO NOT WANT TO or NEED TO chop all the < div > into a separate .html file (I really don't understand why should I have to have 100 .html files?)

http://www.codeply.com/render/DRBmOoChfM

@nateabele
Copy link
Contributor

@eddiemonge Don't we basically have this now with being able to turn strict matching off?

@eddiemonge
Copy link
Contributor

not sure. maybe? would need to test to verify

@stale
Copy link

stale bot commented Jan 24, 2020

This issue has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs.

This does not mean that the issue is invalid. Valid issues
may be reopened.

Thank you for your contributions.

@stale stale bot added the stale label Jan 24, 2020
@stale stale bot closed this as completed Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants