Parameter constraints

[bjyoungblood]

Adds the ability to specify regex constraints for param values on individual routes. Example:

var constraints = { id : /^\d+/, action: /^(create|read|update|delete)$/ };
React.renderComponent((
    <Route handler={SiteWrapper}>
        <Route name='posts' handler={PostPage} path='/posts/:id/:action' constraints={constraints} />
        <Route name='404' handler={NotFoundPage} path='*' />
    </Route>
), document.body);

In this example, the path /posts/123/read will route to PostPage, while the paths /posts/abc/read and /posts/123/foobar will route to NotFoundPage.

[bjyoungblood]

I wasn't sure what exactly to do with links yet. Should they raise an invariant violation if you try to provide params that would be invalid for the specified route name, cause a warning, do nothing?

[mjackson]

I'm honestly not sure about adding this extra level of complexity to specifying routes. It feels like it could create a bunch of corner cases that would be difficult to deal with.

[bjyoungblood]

That's part of the reason I made it optional. The thinking here was that it would be nice to have the ability to specify that you only want your :id parameter to be digits -- that way you don't have to handle redirecting to a 404 (or whatnot) in every single handler.

There are other routers that manage to support this without weird cases (like Express). Is it a problem with my implementation?

[bobeagan]

Here's a simplified example scenario where not having constraints would be tedious.

If I have a route that does something like /foo/* but I need /foo/a, /foo/b, and /foo/c handled by one component and /foo/x, /foo/y, and /foo/z handled by another, then I would either have to explicitly write out those routes (which does not scale) or make a another component that does that logic and acts as a wrapper to load the correct handler.

It seems excessive versus just providing an optional means of accomplishing this from the route list directly. For larger apps, I can't imagine having to make every dynamic data route do the validity check and 404 redirecting.

Maybe I'm missing something though? Love to hear your feedback on efficient alternatives.

[ryanflorence]

I like this feature, I also like narrowly scoped libraries.

If Route were a mixin, are there enough entry points in the code to add this behavior to a new <ConstrainedRoute/> component?

[ryanflorence]

Line 254 of Route.js defaults constraints to {}, why the ! constraints check? Also, constraints === {} will always be false since comparing to objects is always identity.

This code never runs, not even if somebody sets constraints to something falsey because of the way the default is set up in Route.js.

[mjackson]

Most routing libraries (including Sinatra and Express) ultimately just end up providing the user with the ability to supply a regular expression for maximum flexibility. The way this feature is implemented in this PR, users don't just specify a single regular expression, but possibly many that match on a single route. To me, this feels fairly complex.

One of the greatest strengths of the library now is that it's easy to quickly see and understand how your routes are laid out. If you're debugging something, it should take you about 5 seconds to look through your routes and find the component that is responsible for rendering the URL you're having an issue with. If users start using regular expressions to match on URL paths, this task gets significantly more complex very quickly, especially with nested routes.

This feels like one of those areas where we can give users a lot of value by providing constraints based on experience. At this point we've got the ability to match segments of a URL using :propertyName and *. I'd like to see some real-world use cases (i.e. not /foo/a) where this isn't sufficient and see if there are other ways of dealing with the issues there.

(Also, just FYI every feature of this library has been driven by tiny apps found in the examples directory. It's a great way to think through the features we need and see how they hold up in real-world scenarios.)

@rpflorence To answer your question, probably the most obvious place to put a hook into the code would be in Route#match. If users had the ability to specify custom matching behavior, it should be there.

[bjyoungblood]

I think that the chief example would be in an app where you have numeric IDs in your URLs to identify objects in a database. Your routes would be /users/:userId, /posts/:postId, etc.

If you know beforehand that these routes are going to be invalid if alpha or special characters are supplied to userId, then being able to specify this in the routes will prevent you from having to duplicate the logic of sending the user to an 404 (or other error) page in every single handler. That's obviously a valid solution, but to me, it seems like it would be simpler to filter out values that are certain not to work at a higher level.

[ryanflorence]

I'm still not opposed to this, but what about a mixin for route handlers that does a similar thing?

var Constrainable = {
  statics: {
    willTransitionTo: function(transition, params) {
      if (!this.validate(transition.path) || !this.validate(params))
        transition.redirect(this.redirectTo);
    },

    validate: function() {}
  }
};

var SomeHandler = React.createClass({
  mixins: [Constrainable],
  statics: {
    redirectTo: '404',
    pathConstraints: {
      id : /^\d+/,
      action: /^(create|read|update|delete)$/
    }
  }
});

[bjyoungblood]

@rpflorence That seems like it would work well and have more predictable behavior than what I currently have. I'll close this PR and implement that on another.

[ryanflorence]

I still like the idea of configuring validation rules in the route config instead of the handlers because handlers don't care about their paths and can be reused at different paths. But I'd like to make the code expose some entry points that allows us to extend with this functionality, rather than bake it in.

(handlers care about params though, so maybe the handler is the right place after all)