Skip to content
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

Closed
panlina opened this issue Oct 1, 2018 · 5 comments
Closed

Comments

@panlina
Copy link

panlina commented Oct 1, 2018

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

@imcuttle
Copy link

imcuttle commented Oct 27, 2018

@jfhbrook @panlina I have same issue here.
I wrapped ecstatic to solve it temporarily

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)
    })
  }
}

imcuttle added a commit to imcuttle/github-similar-server that referenced this issue Oct 27, 2018
@panlina
Copy link
Author

panlina commented Oct 27, 2018

Really want ecstatic to be able to work in the way requested. But seems @jfhbrook is busy with other projects.

@jfhbrook
Copy link
Owner

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.

@panlina
Copy link
Author

panlina commented Oct 29, 2018

Thank you for taking time to have a look. I'm working on a pull request now.

@panlina panlina changed the title baseUrl does not work correctly when mounted non-root, and is not necessary baseDir does not work correctly when mounted non-root, and is not necessary Oct 30, 2018
panlina added a commit to panlina/node-ecstatic that referenced this issue Oct 30, 2018
@jfhbrook
Copy link
Owner

jfhbrook commented Apr 5, 2019

closing this and tracking with your PR

@jfhbrook jfhbrook closed this as completed Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants