-
Notifications
You must be signed in to change notification settings - Fork 982
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
Support server.use('/route', handler) and server.all() #289
Comments
I think the express way would be to add a |
I think Falco's suggestion is a nicer API - that gives you the same thing - it'll just be a bit weird to go tack that in in the code, but from a library perspective I like that. |
Thanks for pointing that. Much better that way. |
Reopen as being not implemented yet? |
Yes, thanks!. |
+1 for extended |
+1. I was originally using Express for a large, distributed service and was able to link routes within my sub-modules into my main API's routes like so:
Being able to do this with Restify would keep me using Restify. |
Any updates on this? Seems like it should be two separate Github issues:
|
I would love to see the |
The trouble with supporting this pattern is that if you've already added a route to the route list, this new handler would have to be added to the end or beginning, which could make it too late or too early for the expected behavior. When you are forced to build the handler chain for your route, you can ensure that handlers are executed in the expected order. |
+1 |
+1 I'm currently using restify-namespace (https://github.com/mpareja/node-restify-namespace) to accomplish similar functionality. |
Yes I would also like this feature! |
Is this still being considered? Im also using restify-namespace for now to check wether restify could replace an express approach. Is there any other mechanic to let's say expose every route under '/api' for example? Cheers. |
What is the status of this issue? It has been suggested 3 years ago! This is very important because it saves us from doing: api.use(req,res,next) => {
if(req.url.startsWith('')){
//etc
}
} |
I don't think this will be implemented because it adds too much overhead. I love restify because of how basic and barbones it is. I hope it stays that way. |
I totally disagree with @roblav96 |
Or do this: server.post( '/api/public/prekey',
require( './middle/public/throttle.js' ).bind( envThrottle.pre ),
require( './middle/public/bytes.js' ),
require( './routes/public/pre-key.js' )
)
server.post( '/api/public/verifycaptcha',
require( './middle/public/throttle.js' ).bind( envThrottle.pre ),
require( './middle/public/bytes.js' ),
require( './routes/public/verify-captcha.js' )
)
server.post( '/api/public/register',
require( './middle/public/throttle.js' ).bind( envThrottle.pub ),
require( './middle/public/bytes.js' ),
require( './middle/public/verify-prekey.js' ),
require( './routes/public/register.js' )
)
server.post( '/api/public/login',
require( './middle/public/throttle.js' ).bind( envThrottle.pub ),
require( './middle/public/bytes.js' ),
require( './middle/public/verify-prekey.js' ),
require( './routes/public/login.js' )
) It only adds those middlewares to my public routes. |
We are working on a solution to the issue of route inheritance and sharing of common middleware. You can take a look at: restify/conductor. It is still a work in progress, but is a system we actively use to manage our routes and their dependencies. The biggest benefit we get is to have a set of common route middlewares that we can share across many different routes, without having to resort to adding them to every route manually. The API is still in flux, but will hopefully firm up soon. The documentation is very minimal right now, but will be improved over time as well. |
Yes, please check out restify-conductor. It is a different take on the abstraction approach for reusable routes, without explicitly tying them to a specific URL(s). We would love to hear any feedback you may have. |
+1 |
1 similar comment
+1 |
I started converting everything to restify this morning and it went really well until i hit this roadblock. We were previously using the middleware solution for route inheritance as well in express. |
+1000 This is required to do some common logic, please add |
If you're working with simple middleware, you might be able to get away with this 8 line replacement for the function scopeMiddlewareTo(prefix, middleware) {
return function(req, res, next) {
if (req.url.indexOf(prefix) === 0) {
req.url = req.url.slice(prefix.length);
return middleware.call(this, req, res, next);
}
else { return next(); }
};
} And use with: server.use(scopeMiddlewareTo('/prefix', myMiddleware)); This works for very simple middleware that are only checking the |
bump 🤔 |
@tbranyen Why do you slice the req.url? If you wish to have more than one handler on the same prefix, this slice makes that impossible. Good workaround though, thanks. |
+1 Has this been implemented yet? Is anyone working on it? |
Maybe this will help |
+1 |
1 similar comment
+1 |
This is essential, |
+1 - would be great to have for shared middleware used across versioned routes |
+1 - will this feature be added in v5.x ? |
This doesn't work, since, restify only routes through middleware when it knows about a registered route on the prefix. |
@alber70g absolutely correct, that's something that has bitten me many times using Restify. I've since abandoned Restify in favor of Express, but our team would like that to be a choice. I met with current maintainers of Restify @DonutEspresso and @yunong and we all agreed this feature should be implemented. However, we need to spec out the work and figure out a less ad-hoc way of introducing the change. The work here would need to include (at a minimum):
This weekend I'll put together an idea of what could be implemented and mock out the intended behavior. If anyone else wants to help contribute to this effort, please think of potential gotchas and wants along with implementation concerns. |
@tbranyen I'm not sure, but if you'd use a regex on a get,post,put,delete it would trigger all middleware since the regex would potentially match on all requests. Although I think there's a reason why this shouldn't work, as it gives a huge performance penalty on routes that don't exist (probably?). Maybe that's why we wouldn't want it in this way. A structural solution for scoped routes would be better. Maybe all routes nested under a |
There is https://www.npmjs.com/package/restify-router to organize routes/subroutes. |
https://www.npmjs.com/package/restify-prefix-route Use this to add prefix in all routes like... server.pre(applyPrefix('/v1')); |
Edited
Would be very nice to have
server.use('/route', handler)
andserver.all()
functions in restify.Thanks Falco and Mark.
Cheers,
David
The text was updated successfully, but these errors were encountered: