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

expressStaticGzip fail to handle single file #13

Open
FranckFreiburger opened this issue Jul 20, 2017 · 6 comments
Open

expressStaticGzip fail to handle single file #13

FranckFreiburger opened this issue Jul 20, 2017 · 6 comments

Comments

@FranckFreiburger
Copy link

Like serve-static, express-static-gzip should also be able to serve a single file.
eg:
expressApp.use('/service-worker.js', expressStaticGzip('service-worker.js'));

Currently, the error is:

Error: ENOTDIR: not a directory, scandir 'C:\test\dist\service-worker.js'
    at Object.fs.readdirSync (fs.js:909:18)
    at findAllCompressionFiles (C:\test\node_modules\express-static-gzip\index.js:127:24)
    at expressStaticGzip (C:\test\node_modules\express-static-gzip\index.js:28:9)
    at serve (C:\test\myserver.js:344:10)
    ...
@tkoenig89
Copy link
Owner

Hi,

as i wanted to restructure the code of expressStaticGzip for a while now and make it easier to test, i will have a look at your issue while doing so.

At the moment I can not guarantee any timeframe when this will be done, as my main focus is on adding tests to ensure no future changes will break the module.
So you might want to provide a pull request for a fix,in case this is an urgent issue you are depending on to be fixed.

Regards,
Tobi

@FranckFreiburger
Copy link
Author

Ok, it's not very urgent, thanks and good luck with your refactoring !

@FranckFreiburger
Copy link
Author

oops, maybe the issue can keep open...

@JustFly1984
Copy link

There is no point to serve service-worker.js with expressStaticGzip cos you can't gzip it anyway, at least with Webpack's CompressionPlugin.

Does anybody knows the better way to serve * route with 'express-static-gzip' than I pointed below?

app.get('/service-worker.js', (req, res) => {
  res
    .sendFile(
      path
        .resolve(
          __dirname,
          'client',
          'build',
          'service-worker.js'
        )
    )
})

app.use(
  '/',
  expressStaticGzip(
    'client/build'
  )
)

app.use(
  '*',
  expressStaticGzip(
    'client/build'
  )
)

Hope it will be possible to have options to serve single file or array of files, not only the directory.

Thank you for a great plugin!

@tkoenig89
Copy link
Owner

I think you can reduce your last two middleware registrations to just one like this:

app.use(expressStaticGzip('client/build'))

This should handle all requests, that would not have been handled before. But you might want to have some other request handler at this position, to serve "404" errors.

I tried adding the single file functionality a while ago, but it took me quite some time, and i did not figure out a simple solution at that time. As this was not one of my own use cases, i stopped implementing. Any PR providing such functionality would be welcome :)

@kamleshchandnani
Copy link

@FranckFreiburger you could use app.get instead of app.use because app.get gets mounted on the path and you can't server the file

app.get(
    '/service-worker.m?js',
    expressStaticGzip('build/client', {
      index: false,
      enableBrotli: true,
      orderPreference: ['br'],
    })
  );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants