express
express copied to clipboard
expressjs
Please add app.route('foo').use()
For Express 4.x, documentation says "Using app.route() is a recommended approach to avoiding duplicate route naming and thus typo errors." However, since all() and use() are different, it would be nice to be able to call use() on route objects as well, to avoid duplicate route naming.
We decided against this because .use carries a specific meaning with what it does to req.path
Doesn't route do the same? That is, strip the path segment from the url? Or is there some more I'm not seeing?
In any case, does this means that your recommended usage is:
app.use('/app', middleware);
app.route('/app')
.get(function (req, res) { ... });
or
app.route('/app')
.all(middleware)
.get(function (req, res) { ... });
Does the latter make a difference in OPTIONS behavior or something else?
no, app.route() doesn't strip the path segment from the url. only .use() does. up to @defunctzombie if he wants to support this though. it could make the internal code simpler if all middleware used .route() internally, but i'm not sure i see a use-case for this.
I need to check on the OPTIONS behavior but I would lean towards making sure .all doesn't affect OPTIONS for .route or app.all() but that might break some other assumptions people have made (or backwards compat) so it could be a no-go.
The second approach is what I would recommend but it could depend on your reliance on req.path inside the middleware.
Well, at the very least, when enabling all debugging, using .all will add 24 separate handlers. That doesn't seem reasonable for simply using a middleware.
The way I see it is simply that there are three types of handlers on the incoming requests:
- Handlers that simply skip requests not matching path prefix
- Handlers that skip requests not matching path prefix and strip prefix from requests matching path prefix
- Handlers that match a certain METHOD and resolve path containing wildcards, params, etc.
These three types should be usable in all situations.
Yeah, sorry, route() does not strip the path, I confused it with Router - I thought those two were the same thing, with route('/path') just calling app.use('/path', new Router()) and returning the generated instance.
I think we should change .all to simply add middleware and not affect options responses. Yes, it will be slightly different than app.all, but whatever.
I think we should change .all to simply add middleware and not affect options responses. Yes, it will be slightly different than app.all, but whatever.
+1
I'm a fan of the following syntax, since i keep my apps very modularized:
(note: often times a sub app will handle most of it's own routing, so this much indentation wouldn't be necessary irl)
app
.route('/library')
.use(middleware.auth.hasLibraryCard)
.route('/books')
.get(controllers.books.index)
.get('/:book_id',
controllers.books.show)
.route('/overdue')
.get(controllers.overdueBooks.index)
.post(controllers.overdueBooks.create)
.end() // this would be nice to help in chaining
.post(controllers.books.create)
.end()
.route('/shelves')
.route('/:shelf_id')
.use(middleware.loadShelf) // to req.locals or something
.get(controllers.shelves.show)
.post(controllers.shelves.update)
.end()
.end()
.end()
.end()
.route('fire_station')
// etc....
cheers!
easier to make a module that does that for you, then see if people are interested in it. adding it to express means additional bloat.
plus, i hate that .end() stuff. and the ridiculous amount of nesting.
plus, i hate that .end() stuff. and the ridiculous amount of nesting.
:+1:
I think we should change .all to simply add middleware and not affect options responses. Yes, it will be slightly different than app.all, but whatever.
FYI route.all never did affect the options behavior, so there is nothing to change there. route.use is still useful in situations where you want to add a middleware that is expecting the path to be stripped (like static and such).
Sounds like a 5.x feature maybe. Along with possibly ripping out the routing system into a module finally.
Adding route.use should be trivial for 4.x, right? I was thinking of adding it for the 4.5 release.
@nakedible after re-reading your original posts, it is clear you are confusing app.all behavior with route.all behavior: app.all adds 24 handlers and affects OPTIONS, but route.all does not and only adds a single handler.
Anyway, a route.use() that just operates like route.all() but strips the path does not seem useful, because of course the path will always be /. BUT I think route.use() would be useful if it would make the middleware only invoked if there was a handler for the incoming method. This would make it very useful for adding middleware to your route, but if your route only did GET and POST, your middleware wouldn't even be invoked when a TRACE came in on your route for no reason.
Thoughts? @defunctzombie @jonathanong
I think having a route.use() that strips the path could be interesting. It would allow you to built up trees of routes minimizing repeating the same route string. I support either adding it and maintaining the strip path semantics or not adding it.
@defunctzombie you're thinking a signature of route.use(path, fn), right (where path defaults to '/')?
Well, I don't see how that would really build trees, then? Do you have an example? My only example I can think of is
var userRouter = express.Router()
userRouter.route('/:id')
.get(getUser)
.put(updateUser)
app.route('/users')
.get(getUsers)
.post(createUser)
.use(userRouter)
But really all those handle under the route could just be under '/' in the router...?
Actually, the above would never work, because .route(path) matches up the full path, so /user/1 would never come through there.
Yea, actually I think you are right in that it should support (path, fn) if it supports anything at all.
Well, I just realized that route(path) does a full match on the path, so like path to use would essentially only match downwards, like app.route('/users/:id').use('/users', fn) would work, but app.route('/users').use('/users/:id', fn) would not work or something.
Right, the .use would be path after stripping away the route's path. Then it could strip away any additional path you pass as well.
Right, the .use would be path after stripping away the route's path.
What I'm saying is that the route's path is actually a match-to-the-end match, so app.route('/users') would not match /users/1 and such, which seems like a .use() under that would just never match anything to me...
It seems that having this feature is useless then?
Right. So the only use I could come up with was the one above, which would let a user do this:
app.route('/users')
.use(bodyParser.json())
.get(getUsers)
.post(createUser)
and the bodyParser.json middleware would not even be invoked for a PUT /users. It's super easy to implement, but I'm not sure how much of a value it would be. It would basically be the short-hand of doing
app.route('/users')
.get(bodyParser.json(), getUsers)
.post(bodyParser.json(), createUser)
-1 for ".use()" in routing. Its a terminology that confuses people, "use" for middleware and such and nothing else
Why app.route() doesn't have use() when Router() instance has? What's the difference and what is the proper manner to mount subapp/router over app.route() instance? Looks like all has another meaning so I need use to mount whole subapp.
