-
Notifications
You must be signed in to change notification settings - Fork 194
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
baseDir does not work correctly when mounted non-root, and is not necessary #235
Comments
@jfhbrook @panlina I have same issue here. function wrapStatic(opts = {}) {
return function(req, res, next) {
const baseDir = typeof opts.baseDir === 'string' ? opts.baseDir : req.baseUrl
req.url = req.originalUrl || req.url
let fn = ecstatic(
Object.assign({}, opts, {
baseDir
})
)
if (!Array.isArray(fn)) {
fn = [fn]
}
fn.forEach(handle => {
typeof handle === 'function' && handle.apply(this, arguments)
})
}
} |
Really want ecstatic to be able to work in the way requested. But seems @jfhbrook is busy with other projects. |
I don't have a strong opinion. I tend to use ecstatic from the cli these days, and I think most people working w/ express use express's static fileserving middleware. I can take pull requests though! It sounds like you know what the behavior is you want and how to get it. |
Thank you for taking time to have a look. I'm working on a pull request now. |
closing this and tracking with your PR |
Previously I posted issue #233 , but now I think that is not about absolute path/relative path, but the way this middleware works.
The goal may be achieved by using the middleware like this:
app.use('/', ecstatic({.., baseDir: "a"}));
,However, if I use it like this:
app.use('/a', ecstatic({.., baseDir: "a"})); //no matter whether baseDir is set
,it does not work correctly as I addressed in #233 , but using a middleware like that is quite common in express apps.
Think a little further, due to express's mounting feature, as stated in req.originalUrl,
baseDir
might be unnecessary at the 1st place. The user can just mount the ecstatic at the path they want, and let express strip the base for you:app.use('/a', ecstatic({..}));
This works better, because this makes the middleware more portable, in the sense that the middleware does not need to know which path is it working on. The middleware can just assume the
req.url
contains barely the file path, which simplifies things a lot.How do you think? @jfhbrook
The text was updated successfully, but these errors were encountered: