-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
Store matched routes in request #34
Changes from all commits
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 |
---|---|---|
|
@@ -28,23 +28,32 @@ var hasOwnProperty = Object.prototype.hasOwnProperty | |
|
||
module.exports = Layer | ||
|
||
function Layer(path, options, fn) { | ||
function Layer(paths, options, fn) { | ||
if (!(this instanceof Layer)) { | ||
return new Layer(path, options, fn) | ||
return new Layer(paths, options, fn) | ||
} | ||
|
||
debug('new %s', path) | ||
debug('new %s', paths) | ||
var opts = options || {} | ||
|
||
this.handle = fn | ||
this.name = fn.name || '<anonymous>' | ||
this.params = undefined | ||
this.path = undefined | ||
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. Add a line for 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. |
||
this.regexp = pathRegexp(path, this.keys = [], opts) | ||
|
||
if (path === '/' && opts.end === false) { | ||
this.regexp.fast_slash = true | ||
if (paths === '/' && opts.end === false) { | ||
this.fastSlash = true | ||
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. V8 can do optimizations if the shape of an object remains the same. This addition will have only some 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. |
||
} | ||
|
||
this.paths = !Array.isArray(paths) ? [paths] : paths | ||
this.paths = this.paths.map(function (path) { | ||
var pathObj = { | ||
path: path, | ||
keys: [] | ||
} | ||
pathObj.regexp = pathRegexp(path, pathObj.keys, opts) | ||
return pathObj | ||
}) | ||
} | ||
|
||
/** | ||
|
@@ -113,14 +122,23 @@ Layer.prototype.match = function match(path) { | |
return false | ||
} | ||
|
||
if (this.regexp.fast_slash) { | ||
if (this.fastSlash) { | ||
// fast path non-ending match for / (everything matches) | ||
this.params = {} | ||
this.path = '' | ||
this.matchedPath = this.paths[0] | ||
return true | ||
} | ||
|
||
var m = this.regexp.exec(path) | ||
var checkPath, m; | ||
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. All of the express modules are working toward compliance with 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. |
||
|
||
for (var i = 0; i < this.paths.length; i++) { | ||
checkPath = this.paths[i] | ||
if (m = checkPath.regexp.exec(path)) { | ||
this.matchedPath = checkPath | ||
break | ||
} | ||
} | ||
|
||
if (!m) { | ||
this.params = undefined | ||
|
@@ -137,7 +155,7 @@ Layer.prototype.match = function match(path) { | |
var params = this.params | ||
|
||
for (var i = 1; i < m.length; i++) { | ||
var key = keys[i - 1] | ||
var key = this.matchedPath.keys[i - 1] | ||
var prop = key.name | ||
var val = decode_param(m[i]) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -985,6 +985,31 @@ describe('Router', function () { | |
}) | ||
}) | ||
|
||
describe('req.matchedRoutes', function() { | ||
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. Same as comment above, can you make the formatting through here 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. |
||
it('should store matchedRoutes in request', function(done) { | ||
var router = new Router() | ||
var barRouter = new Router() | ||
var bazRouter = new Router() | ||
var server = createServer(router) | ||
var matchedRoutes | ||
|
||
router.use(['/foo/:id', '/foe'], barRouter) | ||
barRouter.use(['/bar'], bazRouter) | ||
bazRouter.get(['/bez', '/baz/:subId'], function(req, res, next) { | ||
matchedRoutes = req.matchedRoutes | ||
next() | ||
}) | ||
router.use(saw) | ||
|
||
request(server) | ||
.get('/foo/10/bar/baz/30') | ||
.expect(200, 'saw GET /foo/10/bar/baz/30', function(err, res) { | ||
assert.deepEqual(matchedRoutes, ['/foo/:id', '/bar', '/baz/:subId']) | ||
done(err) | ||
}) | ||
}) | ||
}) | ||
|
||
function helloWorld(req, res) { | ||
res.statusCode = 200 | ||
res.setHeader('Content-Type', 'text/plain') | ||
|
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.
This is going to create unnecessary arrays. Can you instead only create a new array the first time, then push to it after?
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.
Fixed in https://github.com/pillarjs/router/pull/86/files#diff-168726dbe96b3ce427e7fedce31bb0bcR295