Add support for mapping route params to regexps or functions for processing

[ericf]

It's desirable to both use string-based routes with named params, e.g., "/posts/:id", and assign some processing to route param values, e.g., id should be a number.

This adds a feature to Router to support mapping named params to regular expressions or functions. See the following example:

var router = new Y.Router({
    params: {
        id      : Number,
        username: /^\w+$/
    }
});

router.route('/posts/:id', function (req) {
    // This route will only be executed when `id` is a number.
    // `req.params.id` is number resulting from passing the string value
    // to the `Number()` constructor.
    Y.log('Show post: ' req.params.id);
});

router.route('/users/:username', function (req) {
    // This route will only be executed when `id` is a number.
    // `req.params.username` is the result object/captures array from
    // `exec()`-ing the regular expression.
    Y.log('Show user:' req.params.username[0]);
});

router.save('/posts/1');     // => "Show post: 1"
router.save('/users/ericf'); // => "Show user: ericf"

// This won't log anything because `id` is not a number, the route is skipped.
router.save('/posts/asdf');

// This won't log anything because `username` is not alpha-number, the route 
// is skipped.
router.save('/users/eric,ryan');

Goals

The main goal for this feature is to enable people to stick with string-based route paths and named req.params while adding support for basic param processing/validation.

Currently, when people want to process request parameter values, they either have to add a route callback function, or switch to using regex route paths. Adding another callback/middleware isn't the end of world, but it gets repetitive since it has to be added to every route (or to a common middleware stack). Having to switch to using regex route paths sucks because you lose named request parameters, and those route paths are hard for humans to parse.

Todos

• Write initial implementation
• Add support for functions
• Write tests which fully cover feature
• Test in all YUI Target Environments
• Fill out API docs
• Update user guide
• Add HISTORY.md entry

[saw]

👍

[rgrove]

I kinda agree with @tivac (in IRC) that allowing functions as parameter validators would be useful. Sometimes a regex isn't the best tool.

Other than that, this looks great.

[clarle]

I wasn't present during the IRC conversation, but is this is supposed to be similar to express-params?

https://github.com/visionmedia/express-params

If so, I agree with @rgrove. Doing:

var router = new Y.Router({
    params: {
        id: Number
    }
});

Is a lot better than using regular expressions.

[ericf]

@rgrove @tivac I added a "Goals" section to this PR's description. I think it's reasonable to add support for functions, but what do you guys think should be the "contract" for param functions?

I'm thinking the functions should be sync (i.e. return a usable param value), if you need async use a route callback/middleware function. As for the arguments the function is called with, I'm thinking the param name and its current value parsed from the request's path. This would artificially restrict these functions since during the dispatch process where they are called we have other info like the request object, but again, if you need cross-param validation, use a route callback/middleware function instead.

[ericf]

@clarle so the param function signature would look like: fn( value , [name] ) and return the new param value?

[clarle]

@ericf That looks good to me. 👍

Falsy return values (but not zero) should move on to the next route, like what express-params does.

exports.invalidParamReturnValue = function(val){
  return null == val
    || false === val
    || ('number' == typeof val && isNaN(val));
};

[ericf]

Also, how should the param function's return value be interpreted as failing validation? All falsy values but empty string means failed validation?

[clarle]

Sorry for editing my comment before, but yeah, empty string is fine, and zero is fine.

[ericf]

@clarle oh yeah, 0, good point.

[rgrove]

I agree param functions should be synchronous, and should have the signature fn(value, name).

However, a return value of false seems perfectly legitimate (imagine a boolean param). I suggest treating an undefined or null return value as validation failure, and anything else as success.

[ericf]

@rgrove hmm… I'm not sure which way to side on this. I just pushed a commit that treats a false value as invalid, but I didn't see your comment beforehand.

[ericf]

Should a false value return from a param handler function mean the param value fails validation and therefore the current route should be skipped?

[rgrove]

If only there were a way to change code that's already been written! ;)

[ericf]

@rgrove can you give a real-world-ish example of where a false parameter value would be useful and originate from a URL?

[rgrove]

@ericf: Off the top of my head, I can only think of contrived examples that would probably be bad URL design. So, meh. Do what you think is right.

[tivac]

Supporting bare simple functions like this is a nice improvement over just regexp so I'm happy, but I'm still gonna stump for my favorite alternative.

I'd like to see this create a middleware function that runs before the rest of the route & it can just choose to not call .next() if the test fails. My arguments for using the existing middleware are essentially this.

1. It already exists & has a framework for running a series of potentially async functions.
2. While you can do bad things with that we should mitigate that by warning people away from bad decisions and not having them in examples (unlike Express, apparently).

I think this could be accomplished by changing the signature of the functions to function(value, [name], [done]) & then either calling .next() for the user if it's a sync function or letting them call it when they're done with their async code. If the param function is async they would call done() with a single param or false because (like @rgrove) I can't come up with a scenario where those make any sense as URL params.

[ericf]

@tivac Why do you want support for async param functions?

Just because there's a dispatching infrastructure which supports async functions and we could use it, that doesn't mean we should without a use case. How you would expand the Goals section I wrote in this PR's description to include why async param handling is desirable?

[ericf]

With the fix in #1077, these changes work in all of YUI's Target Environments.

[ericf]

This is ready to be merged!