Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router should accept a route config object in addition to a string or regex path #1067

Closed
ericf opened this issue Aug 7, 2013 · 1 comment
Assignees

Comments

@ericf
Copy link
Member

ericf commented Aug 7, 2013

Currently, Y.Router#route() only accepts a string or regex path as its first argument, it then processes that into a route config object. It would also be useful if Router accepted an already processed route config object provided by the caller.

To coordinate routes between the server (like Express) and client, it's important to make sure subtle things like whether to use optional trailing slashes are preserved. The best way to do this is to provide Y.Router with the same regex path that's used server side. The problem in doing so is when a regex path is used, you have to use unnamed params in route handlers. Allowing the following would resolve this issue:

var routeConfigFromServer = {
    path  : '/posts/:postId',
    keys  : ['postId'],
    regexp: /^\/posts\/(?:([^\/]+?))\/?$/i
};

var router = new Y.Router();

router.showPost = function (req, res, next) {
    // ...
};

router.route(routeConfigFromServer, 'showPost');

This would allow keys to be supplied with the regexp, giving the route handlers named params.

@ghost ghost assigned ericf Aug 7, 2013
ericf added a commit to ericf/yui3 that referenced this issue Sep 21, 2013
This adds support to Router to accept fully-processed route objects.
This is useful when passing one route's routes to another router, and
for accepting route config objects generated by the server and applying
them to a Y.Router instance on the client.

Closes yui#1067
@ericf
Copy link
Member Author

ericf commented Sep 30, 2013

PR #1210 was merged, closing this.

@ericf ericf closed this as completed Sep 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant