-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
App mount path stripped to a of cardinality of 1 #36
Comments
Hey, thanks! You're right -- it's only one-level deep. This is due to the At least for now, this is the intended behavior. We have to calculate the "base" in the cheapest possible while, while being careful not to override or grab a path that would be matched by a parameter or wildcard route. A fair bit of benchmarking went into that What do you think? |
I think that the use case I have is pretty niche from my experience working with different NodeJS-flavor service adapters. Mostly, my use case is being able to expose an app that has been wrapped in another that forces the inner app not to have to reflect the base path added externally. That base path being "/api/vN" and the inner app allowing you to mount "/foo," etc. directly. I can also see it being somewhat troubling to someone to have to create an app specifically to nest. Compare these two snippets: const app = require('polka')();
const foo = require('./mw/foo');
const bar = require('./mw/bar');
const basePath = '/api/v1';
app
.use(`${basePath}/foo`, foo)
.use(`$[basePath/bar`, bar) const polka = require('polka');
const foo = require('./mw/foo');
const bar = require('./mw/bar');
const basePath = '/api/v1';
const app = polka();
const api = polka();
const v1 = polka();
app.use('/api', api);
api.use('/v1', v1);
v1
.use('/foo', foo)
.use('/bar', bar);
The test I ran (https://jsperf.com/compare-slicers) substantiated what you observed I think when you were describing the tests put into that function! I'm actually very surprised Have you tried a mechanism whereby you mount each part of the path on "apps" and move on to the next given no matches? Is it perhaps worth noting that, although faster, the router doesn't handle all of the use cases of Express? I see this: https://github.com/lukeed/polka/blame/master/readme.md#L31, but I'm concerned that maybe that is not descriptive enough. |
Just FYI I'm using Sapper and am encountering this issue (as it uses Polka). Most of the urls we deal with are in a kind of |
Hey, I have local changes made to reflect this, but have not yet pushed it for public use because it's not fully tested yet. For now (pre-1.0) you'd have to do something like this: const call_sub_app = require('./my-app');
polka()
.use('/woo', (req, res, next) => {
if (req.path.startsWith('/woo/yay/')) {
req.originalUrl = req.path;
req.path = req.path.substring(8);
call_sub_app(req, res, next);
} else {
next();
}
}) Admittedly it's not great, but my work has does not use this pattern, so it's not been moved to a priority seat. Soon! |
In the latest prerelease (1.0.0-next.4) it looks like mounting on nested paths is only partially working: The middleware is called for the appropriate requests, but |
@Conduitry So do you mean that in this example: app.use('/api/:version', foo, bar); ...is only stripping Put differently, an incoming request for |
That is what I meant, but I no longer think that that is actually what I was seeing. I think we're good here with the 1.0.0-next.4 but if you could confirm as well, that would be great. |
Looks to be okay @Conduitry – linked the latest commit to here as a follow-up |
Hey, any chance this might be released soon? Last year I ended up using Express instead, but it's noticeably slower. I tried using the |
The |
Opps, sorry my lack of experience with installing version tags. All good now! Fix works, thanks! |
Hey, thanks for writing this. I'm a huge fan of Express/NodeJS web service alternatives.
It looks like the cardinality of the mounting path of each middleware/app is stripped to a limit of 1.
Example:
If I visit localhost:3000/foo/bar/baz, I will get a 404. I think this is because of the value function, it checks for the first slash after the leading. So, if I provide:
/foo/bar/baz
, the first slash is the zeroeth position of the string and the string itself is stripped tofoo
.Is this by design? If this is done in error, it can easily be fixed with some language around how the trailing slash should be considered, or by using
lastIndexOf
. It also works if I just don't consider the value function at all.Additionally, it looks like there is a
mountpath
instance property, but it is not set from the constructor, perhaps this could be used in some way?The text was updated successfully, but these errors were encountered: