-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Add support for named routes #36
base: master
Are you sure you want to change the base?
Changes from all commits
6cce8fc
79a34fd
24c893c
711f0db
fee1d39
c3b3e31
d59097d
1d8f28d
1b4922c
78b0670
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,33 @@ describe('Router', function () { | |
assert.equal(route.path, '/foo') | ||
}) | ||
|
||
it('should set the route name iff provided', function () { | ||
var router = new Router() | ||
var route = router.route('/abc', 'abcRoute') | ||
assert.equal(route.path, '/abc') | ||
assert.equal(route.name, 'abcRoute') | ||
assert.equal(router.routes['abcRoute'], route) | ||
var route2 = router.route('/def') | ||
assert.equal(router.routes['abcRoute'], route) | ||
assert.equal(null, router.routes[undefined]) | ||
}) | ||
|
||
it('should not allow duplicate route or handler names', function () { | ||
var router = new Router() | ||
var route = router.route('/abc', 'abcRoute') | ||
assert.throws(router.route.bind(router, '/xyz', 'abcRoute'), /a route or handler named "abcRoute" already exists/) | ||
var nestedRouter = new Router() | ||
router.use('/xyz', 'nestedRoute', nestedRouter) | ||
assert.throws(router.route.bind(router, '/xyz', 'nestedRoute'), /a route or handler named "nestedRoute" already exists/) | ||
}) | ||
|
||
it('should not allow empty names', function () { | ||
var router = new Router() | ||
assert.throws(router.route.bind(router, '/xyz', ''), /name should be a non-empty string/) | ||
assert.throws(router.route.bind(router, '/xyz', new String('xyz')), /name should be a non-empty string/) | ||
assert.throws(router.route.bind(router, '/xyz', {}), /name should be a non-empty string/) | ||
}) | ||
|
||
it('should respond to multiple methods', function (done) { | ||
var cb = after(3, done) | ||
var router = new Router() | ||
|
@@ -480,7 +507,7 @@ describe('Router', function () { | |
.expect(200, cb) | ||
}) | ||
|
||
it('should work in a named parameter', function (done) { | ||
/* it('should work in a named parameter', function (done) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just marking down the concern that there is a commented-out test in here still :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it does depend on path-to-regexp 1.x so it couldn't be landed until that dependency is updated and I guess this test would be deleted or rewritten as part of that. |
||
var cb = after(2, done) | ||
var router = new Router() | ||
var route = router.route('/:foo(*)') | ||
|
@@ -495,7 +522,7 @@ describe('Router', function () { | |
request(server) | ||
.get('/fizz/buzz') | ||
.expect(200, {'0': 'fizz/buzz', 'foo': 'fizz/buzz'}, cb) | ||
}) | ||
})*/ | ||
|
||
it('should work before a named parameter', function (done) { | ||
var router = new Router() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to make
findPath
a fully-documented public expectation for other router implementations. The idea behind the router currently is that anyone can implement various different ones and mix and match as needed. The docs, in addition to describing this new method, should describe how we would expect other router implementations to implement it as well.We may need to consider what's going to happen if we just call
findPath
on someone's existing handler function that's out there as well, since we've never done that before, so don't know what we'd be breaking. Let's get some research going on this :)