Only route registered routes

[EyLuismi]

Note: Using Riot 2.3.1
So I was rewriting my webapp which used:
http://example.com#media/id

In the new version the base by default is '#' but it affects every link in the webpage, using or not using '#' so I couldn't click any links because it didn't do anything at all. I really needed to get it working so I finally changed the base to '/' and then the url in the adressbar started to change as I clicked the links but, It does nothing, just works with registered routes, in my case '/media...' after changing the base.

To get it working I needed to register '/..' and then redirect by javascript if the destiny was different from the origin. Here is the code (Which right now doesn't think in queries) I used to get this working If it is of any help:

getUrl = ->
  "#{window.location.protocol}//#{window.location.host}#{window.location.pathname}"

oldMediaId = null
baseUrlForRouter = getUrl()

$(document).ready ->
  riot.route.base('/')

  riot.route 'media/*', (mediaid)->
    if mediaid? and oldMediaId != mediaid
      mediaPopupExec(mediaid)
      oldMediaId = mediaid

  riot.route '/..', () ->
    tUrl = getUrl()
    if tUrl != baseUrlForRouter
      window.location.replace(tUrl)

  riot.route.start(true)

So I can't find anything to solve it in a more official way. Usually in ExpressJS (Node.js) routes are like (req, res, next), and the next param is to call the next registered route which fits the actual route with his filter. If it doesn't have any other, it makes the default move, which in Express usually would be 404 because It doen't find any route, but here would be to send it to the clicked url.

In less words: When any route doesn't satisfy the asked route, it ""shouldn't do event.preventDefault()"" (Just as an example XD)

Sorry if there is an official way to do this, but I couldn't find it :S

[shamotj]

We have exactly the same problem, we stuck on this just couple minutes after you post this issue. Desired outcome for us is that router will hande only routes which passes filter (or are registered by router).

[cognitom]

Oh, that's a bug. We got to fix it.
A temporal work-around is using absolute path:

• some/path/ --> http://example.com/#some/path/

[cognitom]

Hi @LDiab @shamotj
I'm working on this issue today. It's not so simpler than what I thought first. Possible strategies here:

• A. if not match base
• B. if not match routes and has no default route
• C. if not match routes and all default routes returns false

As my understanding riot.observable is design to not return the value to the event emitters, so C would be complex.
Is it enough A or B? Thoughts?

[EyLuismi]

Hi, I think B would be the best option, If a route dont satisfy any registered filter it should be ignored by the Route and the browse should take the default option (If I have understand it). So an app who only uses Riot routes if he wants to show a 404 whenever the user enter in a invalid url, he just needs to use '/..', and in server based urls, the server will handle the 404.

Returning false is an interesting option which allows more complex routing and behave, but it can be avoided for now, it can be a "feature request".

Thanks for working in this :)

El 14 nov 2015, a las 7:00, Tsutomu Kawamura notifications@github.com escribió:

Hi @LDiab @shamotj
I'm working on this issue today. It's not so simpler than what I thought first. Possible strategies here:

A. if not match base
B. if not match routes and has no default route
C. if not match routes and all default routes returns false
D. if all routes returns false
As my understanding riot.observable is design to not return the value to the event emitter, so C / D would be complex.
Is it enough A or B? Thoughts?

—
Reply to this email directly or view it on GitHub.

[cognitom]

@LDiab thanks for your quick feedback. If we use /.. as a filter, all url will be caught. It seems not an expected behaviour, doesn't it?

Think this situation:

riot.route.base('/app')
riot.route('/', function() { /* home */ })
riot.route('/news/*', function(id) { /* news */ })
riot.route('/..', function() { /* not found */ })
riot.route.start(true)

• http://example.com/ --> no router will fire
• http://example.com/other/ --> no router will fire
• http://example.com/app/ --> "home" route will fire
• http://example.com/app/news/1 --> "news" route will fire
• http://example.com/app/report/ --> "not found" route will fire

The last case is a problem. Even if there's existing static page, "not found" route will fire. Does it make sense..?

[EyLuismi]

Mmmm yes, because you have explicitly registered every route to trough that if you haven't registered it before, so to solve this the user has two options, manage every url in the webapp with this route, non on the server, or not to register '/..' and handle all other request on the server.

Alternatively maybe could be added a parameter like a callback to go to the next filter, and if there isn't any other filter execute the expected behavior something like this:

riot.route.base('/app')
riot.route('/', function() { /* home */ })
riot.route('/news/*', function(id, next) { /* news */ 
checkByAjaxId(function(newsWithId){
if (!!newsWithId){
/*whatever*/
}else{
next()
}
}) })
riot.route('/..', function() { /* not found OR EXECUTED IF ANY NEWS FOUND */ })
riot.route.start(true)

You know? So It would be always the last argument, and it is executed only if the user wants it, this is how Express.js handle this.

Like I said if the user executes the calback in the last valid filter, then is saying to the route It needs to handle it by server, maybe a next(true) for a direct 'to server' action could be also very interesting.

[cognitom]

Yeah, sounds good. But I have some concern...

• next() fits in async, but riot.route doesn't support it. (at least at this point)
• one route could have multiple callbacks. The order of evaluation would be too complex.

How about just returning false or true from callbacks? The basic concept is almost same, but it doesn't mean that it pass the process to next. More like rejecting, and just giving it up.

[shimaore]

Just to point out that Express' use of next is restricted to applying middleware after a unique route has been chosen. In Express the first route that matches is the only one that gets used. Once the route is selected, then middleware application starts, and that's where next comes into play. (You can't switch to a different route's callback by using next, in other words. You can only skip to the next middleware in the stack for a given route.)

If I understand properly it doesn't seem that riot's router does things differently in terms of route selection: a typical router (Express, Component) loops through the routes, and riot does the same thing as far as I can tell.

(On the other hand, riot uses an observable to dispatch the callbacks, while Express uses a stack of middlewares.)

If I understand properly, what we are trying to achieve here is for a callback to be able to tell the router that it picked the wrong route, and would it please continue to iterate through the list of routes (something Express or other common routers do not support).
EDIT: Actually Express 4.0 supports this through next('route'), see below.
We're also trying to tell click that it should not preventDefault if no route was found, or the route callbacks that were tried all requested to pass.

I think the solution with a simple boolean answers both of these but I'm not sure it's feasible with an observable dispatch, nor that dispatching in random order through an observable is something we want -- if A and B are both callbacks for a given route and A decides that the router should skip the route but B decides to apply changes based on the route, the outcome will be unpredictable.

@cognitom in terms of API, instead of a true/false return value I'd suggest using a context method like this.pass() which would be used to indicate that the router should skip. This would make the behavior backward-compatible for those of us using CoffeeScript.

[cognitom]

@shimaore thanks a lot for your clarification! I have a bit confused about Express. (I'm totally front-end guy in JavaScript...)
I like your idea this.pass(). I'll update the PR #26 tonight ;-)

[EyLuismi]

@shimaore Hi, Yes, It is used by middleware, but, ExpressJs, has as a Coding Standard (Or something like that, sorry my english level isn't high enough) to use Next in every route you register, as they say:

You can provide multiple callback functions that behave just like middleware, except that these callbacks can invoke next('route') to bypass the remaining route callback(s). You can use this mechanism to impose pre-conditions on a route, then pass control to subsequent routes if there is no reason to proceed with the current route.

So you can do something like this:
Register in this order:
'/', '/admin', '/admin/destroyAllData', '/admin/giveAllMoneyToDogsONG'

And then you in '/admin' take control over the user perms:
if (user.isAdmin){next()}else{sendToJail()}

I think is the cleanest way, but yeah, I use a lot Node.js so all my thoughts are async...

EDIT:

Sorry I maybe dont explain it quite well, basically with the last example I try to demonstrate that it is really useful, if you want to enter:
/admin/destroyAllData
or
/admin/giveAllMoneyToDogsONG

Because when you enter on one of this two routes, before it hits them, you hit /admin where you check if he is able to enter there, so If he can enter, then you give he access by calling next() either way you send it to a 401 page or redirect them to the landing page or whatever you need.

Also, now I see "another problem" it would be awesome to have something like this.getNextRoutes() or something like that to get what would be the next routes if next was called because, if there isn't any route, then you are actually entering in '/admin' if it is not, then you can call next(), this is to render '/admin' after checking for example you are admin.

But this would be another 'issue' here in Github I think only if 'more than one route can be hit'

Thanks for everything 👍

[shimaore]

@LDiab I stand corrected, I missed the introduction of next('route') in Express 4.0, which does what we're trying to do here (i.e. in Express skip the remainder of the middleware stack and process the next route). Your example should read

if (user.isAdmin){next('route')}else{sendToJail()}

to use that feature. (In previous versions one would have used that middleware in the stack for /admin, /admin/destroyAllData, /admin/giveAllMoneyToDogsONG etc.)
Also edited my answer above to correct this.

[EyLuismi]

@shimaore No, by adding 'route' you are doing the opposite:

except that these callbacks can invoke next('route') to bypass the remaining route callback(s)
source: http://expressjs.com/4x/api.html#app.METHOD

Executing next() will get to the next route. I use it all the time. Thats why I found it so interesting to insert it here or a similar.

[EyLuismi]

But I think we are messing here, there is two things in this Github issue, so I think It should be splitted:

• Not registered urls should not preventDefault()
• Hit more than one registered url (Which is my suggestion)
  • A list in the context with the registered urls that will be hit if you keep doing next() or whatever

So here would be only the first item. Any thoughts?

[cognitom]

Yeah, at this time, I'd like to merge #26 without next(), once. And keep this issue open.

[shimaore]

Alright, apparently I've been spending too much time in middleware and didn't really understand how Express behaves wrt the last callback in a route stack.
I tested this and things inside the last callback of the route stack (which is the only one most people use, and the only case riot really cares about today) behave as described by @LDiab with the exception that /admin won't catch the requests below its path, you would need to use /admin/* for that.

In the order things are processed:

• Middleware are the functions registered with app.use. They are "mounted" under a given path (by default /) and are executed for all requests that use that path as a prefix. They get executed in order, and inside a middleware function next() will pass control to the next middleware, or to the callback stack if the last middleware was reached. next('route') inside middleware has no special handling, it sends 'route' back to the client.
• Routes are processed in the order they are registered using app.get, app.post, ... and the first route that matches will execute its callback stack. The callback stack are the functions registered for a specific verb (method) and route pattern.
  • In callbacks except the last callback, next() will pass control to the next callback in the stack (same as in middleware). next('route') will skip the remainder of the callback stack and try the next route.
  • In the last callback of the callback stack for a given route, next() will try the next route. next('route') behaves the same way as next().
• In all cases, next(new Error(...)) will reject the request and stop processing.
• Not calling next() in a middleware or callback will stop processing of the middleware/callback chain.

[EyLuismi]

Hi, I think the fix isn't working... I still get the url changed but nothing happens and I have only one url registered like this:

var oldMediaId;

oldMediaId = null;

$(document).ready(function() {
  riot.route.base('/');
  riot.route('/media/*', function(mediaid) {
    if ((mediaid != null) && oldMediaId !== mediaid) {
      mediaPopupExec(mediaid);
      return oldMediaId = mediaid;
    }
  });
  return riot.route.start(true);
});

Its the compiled version of my coffeescript, am I doing something wrong? Thanks!

[Pysta]

Fix works for me, here's a snippet from my code, maybe it'll help:

function changePage(id){
    if (id){
        self.pagenum = id-1;
    } else {
        self.pagenum = 0;
    }
    self.page = paginate(self.pagenum);
    self.update();
}
riot.route.base('/admin/user-list');
riot.route.start(true);
riot.route('#*',changePage);
riot.route('',changePage);

[senica]

Would something like angular does be unacceptable in terms of clicking on links? Any link that specifies a target attribute is not handled by the history API (i.e. <a href="/test" target="_self" />). I think I'll add this exception for my own project as I only have two hard links in my app — login and logout.

[tscok]

@LDiab @cognitom I posted about a similar issue in the Riot Forum today but maybe I should have filed an issue about it here instead. Well, since this one is still open I thought I could share my experience (and solution).

What I experienced was that links with relative paths like /some/folder/file.jpg triggered the Router if I was standing on a route like #profile, but not if I was standing in the app root /#. To me it seemed as if I had to tell the router to disregard from "routes" that didn't begin with #.

My solution was to define a base route riot.route.base('/#') - the only thing different from the default base is the forward slash /. With this solution I have no problems linking to files using relative paths.

A side note: using target="_blank" on links also solved my problem, but sometimes you want to be able to link to files in either way.

[bradrydzewski]

@senica I ran into the same issue and ended up using target="_top" as a workaround. This opens the link in the full body of the current window, as opposed to a new tab or window. This should behave very similar to _self as long the page is not being viewed inside a frame.

[GianlucaGuarini]

Closing this issue because it's related to an old router version. Please update to the latest @riotjs/route version if you can.