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

Route matching is not falling back to parametric route #104

Closed
IpaliboWhyte opened this issue Dec 12, 2018 · 7 comments
Closed

Route matching is not falling back to parametric route #104

IpaliboWhyte opened this issue Dec 12, 2018 · 7 comments

Comments

@IpaliboWhyte
Copy link
Contributor

IpaliboWhyte commented Dec 12, 2018

I'm running Restify 7.x in production and I was able to reproduce an issue I discovered down here.

Consider the following:

const http = require('http')
const router = require('find-my-way')()

router.on('GET', '/a/bbbb', (req, res) => {
  res.end('{"message":"hello world"}')
})

router.on('GET', '/a/bbaa', (req, res) => {
  res.end('{"message":"hello world"}')
})

router.on('GET', '/a/babb', (req, res) => {
  res.end('{"message":"hello world"}')
})

router.on('DELETE', '/a/:id', (req, res) => {
  res.end('{"message":"hello world"}')
})

console.log(router.find('DELETE', '/a/bar')); 
// => { handler: [Function], params: { id: 'bar' }, store: null }

console.log(router.find('DELETE', '/a/bbar')); 
// => null

console.log(router.prettyPrint());
// └── /
//     └── a/
//         ├── b
//         │   ├── b
//         │   │   ├── bb (GET)
//         │   │   └── aa (GET)
//         │   └── abb (GET)
//         └── :id (DELETE)

const server = http.createServer((req, res) => {
  router.lookup(req, res)
})

server.listen(3040, err => {
  if (err) throw err
  console.log('Server listening on: http://localhost:3040')
})

If router.on('GET', '/a/bbaa',...) is taking out, DELETE /a/bbar seems to be matched otherwise, I get a 404. Any ideas why this is happening?

Thanks in advance!

@delvedor
Copy link
Owner

Hello! I'll look at this as soon as I have time.
If you have time to try to debug it, be my guest! :)

@mcollina
Copy link
Collaborator

cc @hekike

@IpaliboWhyte
Copy link
Contributor Author

IpaliboWhyte commented Dec 13, 2018

Had a look and it looks like parametricBrother only gets propagated to the first level STATIC children and not all STATIC descendants in Node.prototype.addChild.

@mcollina
Copy link
Collaborator

Would you like to send a PR?

@IpaliboWhyte
Copy link
Contributor Author

Yes, currently working on it.

@IpaliboWhyte
Copy link
Contributor Author

Found another scenario, this is actually from the routes defined in example.js file.

const router = require('./')()

router.on('GET', '/test', (req, res, params) => {
  res.end('{"hello":"world"}')
})

router.on('GET', '/:test', (req, res, params) => {
  res.end(JSON.stringify(params))
})

router.on('GET', '/text/hello', (req, res, params) => {
  res.end('{"winter":"is here"}')
})

console.log(router.find('GET', '/testy'));
// => null

I'm not 100% sure this is happening for the same reason, will take a look into this.

@IpaliboWhyte
Copy link
Contributor Author

Closing this as issue is now fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants