Created
July 2, 2014 02:12
-
-
Save tipiirai/a1f2d1f203de0b5f45a1 to your computer and use it in GitHub Desktop.
Riot.js full router
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(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; | |
}; | |
})(); |
I this is good, I will point some issues in your code above butI think it is better if you implement the new router.
- In javascript it is a good practice to define all
var
at the top of the function because that helps the compiler. - 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 repeateriot.route(...)
over and over... - 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. - 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. - why do you add
match
to the list of keys? - 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);
. - 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.
- IMHO the
emit
and theon
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. - A ternary operator inside a ternary operator is pretty confusing (to me) I suggest that the last one should at least wrapped in parenthesis "()".
- Please use "===" :P
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.
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
Usage: