-
-
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
Routers with Isolated middleware stack #38
Comments
What if two Routers are mounted like such:
If a request comes in at |
@demaius You could have it so |
@demalus the behavior would remain the way it is currently for that case - the router loaded earlier will get to process the request object first. This proposal is limited to the behavior of pathless routes defined in routers. |
Hi @hacksparrow, there is no concept at all for "pathless routes" in Express or this router. Just because the |
@dougwilson I didn't mean pathless literally, it was more about the interface when omitting the path parameter. Sorry about the confusion. So, clarifying myself here: This proposal is limited to the behavior of routes defined in routers without specifying a path. |
Right, but that's the problem: unless there is another change before this to the concepts in the router, app.use(fn) must behave identically to app.use('/', fn) especially because there are many configuration-based builders on top of this module that always provide that argument since it is promised to work in an identical way. |
How about "behavior of routes defined in routers without specifying a path or at the root path"? |
Another major problem with the proposal as-is is request URL rewriting. A middleware is absolutely allowed to alter the URL and method of a request, and processing will continue with the new URL. Because of this, we cannot even understand if a request will match any given route until all prior middleware are executed. |
Agreed. |
Agree with @dougwilson . There's no way to know if you are "inside" of a router when executing middleware. I also agree that single stack can be confusing for developers that haven't experienced it before. Looking at the original ticket #2760, I'm not entirely sure a solution would work. If you have multiple Routers mounted at the same spot, the middleware will be executed for all of them until some piece of middleware exits the chain (usually a route). You could isolate the routers explicitly by doing something like the following:
This doesn't solve the problem either, because middleware is already implicitly isolated because each middleware is checked against Router.path + middleware.path. The real problem I think is that people are expecting it to work like such: if a route is matched in a Router, then execute everything in the router. You could modify what I wrote above to change the match to also loop through all the routes in a router and see if one matches. |
Does anyone know if this kind of feature ever got anywhere? The existing behavior was totally unexpected and derailed a refactor I was working on 😒 |
@QuotableWater7, I do not believe so. I believe the current thinking on this would be, Express does not need to support every feature or configuration, but it should make it simple to swap in parts to get what you need. So in this case, I would recommend using a router which supports the feature you need (router level isolation). If this means forking this to provide your features, go for it. The router is decoupled from much of express as long as you implement the interface. IMHO, we should probably close this issue since it has not seen progress and is quite a big change in how the router works. |
would be really nice to support this:
|
OMG I was facing the same issue, it took me over an hour to get here. This is definitely not the expected behavior and causes lot of confusion. Has there been any progress? |
Router.use() is based on path, not router instance. This means routerA can `.use` a middleware and end up effecting routerB. The user router (and all others) was running through the requireToken middleware as a result. This is a documented issue with express router: pillarjs/router#38
The single stack middleware system of express seems to be a source of confusion for some developers. This is a proposal to isolate the middleware stack in a router from other routers mounted on the same app.
Current behavior:
Proposed behavior:
The current behavior of
app.use()
, which executes its middleware for all the requests to the app would remain the same and should be used for defining global middleware.The text was updated successfully, but these errors were encountered: