Skip to content

Instantly share code, notes, and snippets.

@tipiirai
Created July 2, 2014 02:12
Show Gist options
  • Star 2 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save tipiirai/a1f2d1f203de0b5f45a1 to your computer and use it in GitHub Desktop.
Save tipiirai/a1f2d1f203de0b5f45a1 to your computer and use it in GitHub Desktop.
Riot.js full router
(function() {
var routes = [];
function on(route, fn) {
var keys = ['match'];
route = route.replace(/[\/\=\?\$\^]/g, '\\$&').replace(/\{(\w+)\}/g, function(match, key) {
keys.push(key);
return '(\\w+)';
});
routes.push({ re: RegExp(route), keys: keys, fn: fn });
}
function emit(path) {
for (var i = 0, route, match; (route = routes[i]); i++) {
match = route.re.exec(path);
if (match) {
for (var j = 0, params = { path: path }, val; (val = match[j]); j++) {
params[route.keys[j]] = val;
}
route.fn(params);
}
}
}
riot.route = function(arg, fn) {
fn ? on(arg, fn) : typeof arg == 'function' ? on(".*", arg) : emit(arg);
return riot;
};
})();
@tipiirai
Copy link
Author

tipiirai commented Jul 2, 2014

Usage:

// complex params
riot.route("/items?search={q}&extra={extra}", function(params) {

// match subpath
}).route("search={q}", function(params) {
   // --> params.match, params.path, params.q

// match always
}).route(function(params) {
  console.info("always:", params);

});

@3den
Copy link

3den commented Jul 2, 2014

I this is good, I will point some issues in your code above butI think it is better if you implement the new router.

@3den
Copy link

3den commented Jul 2, 2014

  1. In javascript it is a good practice to define all var at the top of the function because that helps the compiler.
  2. can you enable a sintax like riot.route({"/bla": function(params) { ... }, "/bla2": function () {...}}); this sintax lets you define several routes at once so there is no need to repeate riot.route(...) over and over...
  3. This typeof arg == 'function' ? on(".*", arg) will allow you to define only one route "generic" route at a time, I think that is ok but the older implementation would allow you to to have as many as you want.
  4. If you define a generic route riot.route(function(params){...}) all other routes defined after it will never be executed because the because the generic route will be catching everything.
  5. why do you add match to the list of keys?
  6. You need to make riot route an observable so you can bind it to the browser and allow it to be extended, trigger an "emit" event after the route.fn(params);.
  7. you need to make sure that the tests i wrote for the router pass, my solution was fully test driven I think that is a good practice.
  8. IMHO the emit and the on function are doing too much, i would extract the regex creation and the params generation to a to other private functions, following the single responsibility principle.
  9. A ternary operator inside a ternary operator is pretty confusing (to me) I suggest that the last one should at least wrapped in parenthesis "()".
  10. Please use "===" :P

@3den
Copy link

3den commented Jul 2, 2014

Besides those comments, It was cool that you used an array instead of an object for the routes that simplified a lot the only down side is because all routes will need to loop on the list there is no more O(1) in the best case scenario, but that is not an issue because the most routes will have params or wildcard.

@3den
Copy link

3den commented Jul 2, 2014

wow i noticed that issue 3 wont happen because you will match all routes... I really like that ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment