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

Argument order #25

Open
4 tasks
damianstasik opened this issue Aug 15, 2016 · 1 comment
Open
4 tasks

Argument order #25

damianstasik opened this issue Aug 15, 2016 · 1 comment

Comments

@damianstasik
Copy link

If you are using this module with Express, you can create a named route like that:

app.get('/user/:id', 'user', (req, res) => {
    res.send('Hello!');
});

It's clean and neat, but is you are using standalone mode and want to create the exact route, you end up with that:

router.add('get', '/user/:id', (req, res) => {
    res.send('Hello!');
}, { name: 'user' });

It feels bloated, but wait, what if you want some middlewares?

router.add('get', '/user/:id', [isAuthenticated, someOtherMiddleware, (req, res) => {
    res.send('Hello!');
}], { name: 'user', caseSensitive: true });

My proposal would be to:

  • Change order of the options argument, so it would be after the path.
  • Allow options to be string, then it would be used as a route name.
  • Use the arguments object, so if there would be more than one middleware, then putting them in an array would be optional, like in Express.
  • If the options argument would be a callback, then instead of slicing three args from the arguments object, it would slice two, so options would be optional.

All of the changes above would allow to create a named route like that:

router.add('get', '/user/:id', 'user', isAuthenticated, someOtherMiddleware, (req, res) => {
    res.send('Hello!');
});

router.add('get', '/post/:id', (req, res) => {
    res.send('Hello!');
});

I can create a pull request, but first I would love to know your opinions.

@alubbe
Copy link
Owner

alubbe commented Aug 19, 2016 via email

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

2 participants