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

Updated to path-to-regexp@3.0.0 #42

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,64 @@ router.route('/pet/:id')
server.listen(8080)
```

## Migrating to 2.x from 1.x

The main change is the update to `path-to-regexp@2.0.0`, which has a few breaking changes:

#### No longer a direct conversion to a RegExp with sugar on top.

It's a path matcher with named and unnamed matching groups. It's unlikely you previously abused this feature,
it's rare and you could always use a RegExp instead. An example of this would be:

```javascript
// Used to work
router.get('/\\d+')

// Now requires matching group
router.get('/(\\d+)')
```

#### All matching RegExp special characters can be used in a matching group.

Other RegExp features are not supported - no nested matching groups, non-capturing groups or look aheads
There is really only one common change is needing replacing any routes with `*` to `(.*)`. Some examples:

- `/:user(*)` becomes `/:user(.*)`
- `/:user/*` becomes `/:user/(.*)`
- `/foo/*/bar` becomes `/foo/(.*)/bar`

#### Parameters have suffixes that augment meaning - `*`, `+` and `?`. E.g. `/:user*`

Needs more info.

#### Named params with regex no longer define positionally.

One other small change (hopefully low impact), is that named parameters with regular expressions no longer result in positional
values in the `params` object. An example is:

```javascript
router.get('/:foo(.*)')

// old GET /bar
console.log(req.params) // {0: 'bar', 'foo': 'bar'}

// new GET /bar
console.log(req.params) // {'foo': 'bar'}
```

#### Partial matching, prefer escaping delimiter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section shouldn't be relevant anymore. However, you can make the example /user{s}? if you wanted to.


The update to `path-to-regexp@3` includes a change to how partial matches are handled,
now you should escape the delimiter before a partial match segment. For example:

```javascript
// old
router.get('/user(s)?/:user/:op')

// new
router.get('\\/user(s)?/:user/:op')
```

## License

[MIT](LICENSE)
Expand Down
18 changes: 16 additions & 2 deletions lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,28 @@ var hasOwnProperty = Object.prototype.hasOwnProperty

module.exports = Layer

function Layer(path, options, fn) {
function Layer(p, options, fn) {
if (!(this instanceof Layer)) {
return new Layer(path, options, fn)
return new Layer(p, options, fn)
}

debug('new %o', path)
var opts = options || {}

// If not in strict allow both with or without trailing slash
var path = p
if (!opts.strict) {
if (!Array.isArray(path) && path !== '/' && path[path.length - 1] === '/') {
path = path.substr(0, path.length - 1)
} else {
for (var i = 0; i < path.length; i++) {
if (path[i] !== '/' && path[i][path[i].length - 1] === '/') {
path[i] = path[i].substr(0, path[i].length - 1)
}
}
}
}

this.handle = fn
this.name = fn.name || '<anonymous>'
this.params = undefined
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"debug": "3.1.0",
"methods": "~1.1.2",
"parseurl": "~1.3.2",
"path-to-regexp": "0.1.7",
"path-to-regexp": "^3.0.0",
"setprototypeof": "1.1.0",
"utils-merge": "1.0.1"
},
Expand Down
4 changes: 2 additions & 2 deletions test/req.params.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('req.params', function () {
})
})

router.get('/*', hitParams(1))
router.get('/(.*)', hitParams(1))

request(server)
.get('/buzz')
Expand All @@ -156,7 +156,7 @@ describe('req.params', function () {
})
})

router.get('/*', hitParams(1))
router.get('/(.*)', hitParams(1))

request(server)
.get('/bar')
Expand Down
20 changes: 10 additions & 10 deletions test/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ describe('Router', function () {
it('should work following a partial capture group', function (done) {
var cb = after(2, done)
var router = new Router()
var route = router.route('/user(s)?/:user/:op')
var route = router.route('\\/user(s)?/:user/:op')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good to revert this.

var server = createServer(router)

route.all(sendParams)
Expand Down Expand Up @@ -716,7 +716,7 @@ describe('Router', function () {

it('should capture everything with pre- and post-fixes', function (done) {
var router = new Router()
var route = router.route('/foo/*/bar')
var route = router.route('/foo/(.*)/bar')
var server = createServer(router)

route.all(sendParams)
Expand All @@ -728,7 +728,7 @@ describe('Router', function () {

it('should capture greedly', function (done) {
var router = new Router()
var route = router.route('/foo/*/bar')
var route = router.route('/foo/(.*)/bar')
var server = createServer(router)

route.all(sendParams)
Expand All @@ -740,7 +740,7 @@ describe('Router', function () {

it('should be an optional capture', function (done) {
var router = new Router()
var route = router.route('/foo*')
var route = router.route('/foo(.*)')
var server = createServer(router)

route.all(sendParams)
Expand All @@ -753,7 +753,7 @@ describe('Router', function () {
it('should require preceeding /', function (done) {
var cb = after(2, done)
var router = new Router()
var route = router.route('/foo/*')
var route = router.route('/foo/(.*)')
var server = createServer(router)

route.all(sendParams)
Expand All @@ -770,23 +770,23 @@ describe('Router', function () {
it('should work in a named parameter', function (done) {
var cb = after(2, done)
var router = new Router()
var route = router.route('/:foo(*)')
var route = router.route('/:foo(.*)')
var server = createServer(router)

route.all(sendParams)

request(server)
.get('/bar')
.expect(200, {'0': 'bar', 'foo': 'bar'}, cb)
.expect(200, {'foo': 'bar'}, cb)
dougwilson marked this conversation as resolved.
Show resolved Hide resolved

request(server)
.get('/fizz/buzz')
.expect(200, {'0': 'fizz/buzz', 'foo': 'fizz/buzz'}, cb)
.expect(200, {'foo': 'fizz/buzz'}, cb)
})

it('should work before a named parameter', function (done) {
var router = new Router()
var route = router.route('/*/user/:id')
var route = router.route('/(.*)/user/:id')
var server = createServer(router)

route.all(sendParams)
Expand All @@ -799,7 +799,7 @@ describe('Router', function () {
it('should work within arrays', function (done) {
var cb = after(3, done)
var router = new Router()
var route = router.route(['/user/:id', '/foo/*', '/:action'])
var route = router.route(['/user/:id', '/foo/(.*)', '/:action'])
var server = createServer(router)

route.all(sendParams)
Expand Down