-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
New API for specifying http method #43
Comments
I think I'm leaning towards number 3. It's the most consistent with the way handlers are currently bound to routes. Is there a reason |
I am also leaning to number 3, I actually modeled it a bit after a Go library that I thought had a nice api, gorilla mux. But after looking at these again and writing some use case examples I think it would be better to do something like:
Then The reasoning for not including the handler function in the methods call, IMHO, is that it mixes two pieces of functionality together that do not strictly belong together. Also, it would have to take multiple middleware to be comparative to the current var router = new Router();
var gets = router.methods('GET');
gets.use(function cacheThings () {});
gets.use('/', function () {});
gets.use('/:foo', function () {});
var posts = router.methods('POST');
posts.use('/:foo', function () {});
posts.use('/:bar', function () {}); EDIT: For reference here is the current way of doing the above example: var router = new Router();
router.get(function cacheThings () {});
router.get('/', function () {});
router.get('/:foo', function () {});
router.post('/:foo', function () {});
router.post('/:bar', function () {}); |
So the only issue I have with that syntax is the method name "use". That method name has always been used to mean only the path prefix will be matched, and it will not be an exact path match (thus it's notable absence from the .route() stuff). I don't think we should use ".use()" for any usage that matches an exact path. |
Ok, I can see that for sure. Are you recommending going back to a new method called |
I'm not sure. I've been racking this around my head since you opened the issue, on those APIs. I've been trying to think of it this way: If I had the following code app.route('/user/:id')
.get(function () { ... })
.delete(function () { ... })
.put(function () { ... }) What would it look like if hypothetically these were all custom methods? I' don't have any good answers off the top of my head yet, which is the great part about having a discussion issue :D |
For other references, here are how that golang lib I mentioned works: http://www.gorillatoolkit.org/pkg/mux I think your example could look something like: app.route('/user/:id')
.methods('GET').handler(function () {})
.methods('DELETE').handler(function () {})
.methods('PUT').handler(function () {}) Another option, and IMO worse idea, is this: app.route('/user/:id')
.methods({
'GET': function () {},
'DELETE': function () {},
'PUT': function () {},
}) The reason I think it is "worse" is that it is a huge departure from the current api. But I figured it is just another discussion point... |
At my current job we have our own wrapper over express which exposes an api like so: app.route(method, path, handler) e.g. app.route('get', '/greet/:name', (res, res) => {
res.send(`Hello ${req.params.name}`)
}) I think it looks really neat and it have been working great for us... |
What do you think of this: Along the lines of
or
Technically, |
Starting with a discussion in #41, we need to come up with a new api for specifying the http method[s] for a handler. Here are a few examples:
router.methods('GET', 'POST').use('/', function () {});
router.use(['GET', 'POST'], '/', function () {})
router.route('/').methods('GET', 'POST').handler(function () {})
router.route(['GET', 'POST'], '/').handler(function () {})
All of these are fairly consistent from the current API in my opinion, despite being breaking changes in a few cases. Numbers 1 & 3 are not breaking changes AFAIK since they are just adding new methods. Thoughts?
The text was updated successfully, but these errors were encountered: