-
-
Notifications
You must be signed in to change notification settings - Fork 16.8k
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
Please add app.route('foo').use() #1980
Comments
Suggested tags: 4x, Feature Request, Routes. |
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, |
I need to check on the OPTIONS behavior but I would lean towards making sure The second approach is what I would recommend but it could depend on your reliance on |
Well, at the very least, when enabling all debugging, using The way I see it is simply that there are three types of handlers on the incoming requests:
These three types should be usable in all situations. |
Yeah, sorry, |
I think we should change |
+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)
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 |
👍 |
FYI |
Sounds like a 5.x feature maybe. Along with possibly ripping out the routing system into a module finally. |
Adding |
@nakedible after re-reading your original posts, it is clear you are confusing |
Anyway, a 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 |
|
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 |
Actually, the above would never work, because |
Yea, actually I think you are right in that it should support |
Well, I just realized that |
Right, the |
What I'm saying is that the route's path is actually a match-to-the-end match, so |
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 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 |
looks like a nice discussion around router abstraction, but with no forward path, or convergence. Is closing the right thing to do here? |
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.
The text was updated successfully, but these errors were encountered: