lukeed / trouter

:fish: A fast, small-but-mighty, familiar fish...errr, router*
MIT License
634 stars 23 forks source link

intended behavior question #16

Closed trevh3 closed 4 years ago

trevh3 commented 4 years ago

A scenario based on the README example..

const Trouter = require('trouter');

const router = new Trouter();

router.get('/users/:id', (val) => {
  console.log(`Getting user ${val}`);
});

router.get('/users/123', (val) => {
  console.log(`Getting user 123, but still have access to ${val}`);
});

let obj = router.find('GET', '/users/123');

obj.handlers.forEach(fn => {
  fn(obj.params.id);
});

Result:

% node index.js
Getting user 123
Getting user 123, but still have access to 123

This is a pretty contrived example, but if two routers have a different set of params, what is the correct behavior? In this example, the second route still has access to the param, even though a param was not specified. I was guessing that it should return the first route and ignore the second route.

lukeed commented 4 years ago

Hey,

The order of your defined routes is the order of lookup. In this case, /users/123 will match both /users/:id and /users/123, with /users/:id coming first. This is because it was defined first – ergo, it will be checked first.

If you want /users/123 to be matched first, then your router.get('/users/123') has be defined first.

Trouter returns all matching routes. It's up to you to loop through the results & decide how/when to stop. Here's how Polka does it..

In your example above, you allow the execution to continue. And since params is part of the return result, it's shared across all handlers. Here's another example that might make sense within the Polka/Express mindset:

// use(), but same thing w/ get()
router.use('/users/:id', (req, res, next) => {
  console.log('found user:', req.params.id);
  req.hello = 'world';
  next();
});

router.get('/users/123', (req, res) => {
  console.log('hello?', req.hello);
  //=> "world"
  console.log('params?', req.params);
  //=> { id: "123" }
});

Hope that helps clear things up! If not, don't hesitate to follow up 👍

trevh3 commented 4 years ago

That all makes sense. My question was more directed at the idea that params are shared across all the handlers.

Another (contrived) example:

const Trouter = require('trouter');

const router = new Trouter();

router.get('/:user/:road', (req) => {
  console.log(`user: ${req.params.user}, road: ${req.params.road}`);
});

router.get('/:road/:id', (req) => {
  console.log(`road: ${req.params.road}, id: ${req.params.id}`);
});

let obj = router.find('GET', '/superduper/123');

obj.handlers.forEach(fn => {
  // fake request
  const req = {};
  req.params = obj.params;

  fn(req);
});

Result:

% node index.js
user: superduper, road: superduper
road: superduper, id: 123
lukeed commented 4 years ago

Again, that's intentional.

Trouter matches everything, and part of the match involves parsing parameter value segments.

Conceptually, this is expected. And realistically, a route will be reach for a param key if it defined it/expects it to be there.

trevh3 commented 4 years ago

Even if the param's value is incorrect? In the previous example, the correct answer would be:

% node index.js
user: superduper, road: 123
road: superduper, id: 123
lukeed commented 4 years ago

Yes, because it's your fault

lukeed commented 4 years ago

It's as problematic as having defined /:road/:road – it's your application, you should organize and/or separate routes accordingly.