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

Serve-static option: 'extensions' #539

Conversation

bitjson
Copy link
Contributor

@bitjson bitjson commented Mar 27, 2015

Allows the use of serve-static's extensions option.

extensions

Set file extension fallbacks. When set, if a file is not found, the given extensions will be added to the file name and search for. The first that exists will be served. Example: ['html', 'htm'].

This adds the option to utils.addBaseDir, adds a cli option, and adds the option to e2e.cli.server.js.

usage

browserSync({
    server: {
      baseDir: 'src',
      extensions: ['html', 'json']
    }
  });

cli usage

$ browser-sync start --server build --extensions 'json'

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 99.84% when pulling aee5c98 on bitjson:feature/static-server-extensions-option into 7ff128e on BrowserSync:master.

@shinnn
Copy link
Contributor

shinnn commented Mar 29, 2015

@bitjson Thanks for the contribution.

Yes, we can easily add serve-static's option, but this looks bad practice for me. In this case you can launch a new server which works as you expected and use browser-sync with proxy mode. I want browser-sync built-in server to behave just like a simple static server.

@bitjson
Copy link
Contributor Author

bitjson commented Mar 29, 2015

@shinnn, thanks for the awesome project. I definitely agree with keeping browser-sync's static server as simple as possible.

I think this falls in the area browser-sync should support "out of the box" because it's a standard option of serve-static. It seems unnecessary for projects to have to re-include a separate instance of serve-static in proxy mode because browser-sync doesn't pass through some of its options. I agree though that individually mapping more of serve-static's options is bad practice.

I wanted to avoid make too many changes for this pull request (I think this is important functionality, and I'd love to see it included sooner than a more significant change would allow), but I think a future pull request could simplify everything by exposing serve-static's options and making browser-sync indifferent to the specifics. Since serve-static is very commonly used, it would probably also be more accessible for new browser-sync users to:

"see the serve-static readme for full documentation on browser-sync's static server."

If there's any interest (?), I could work on that in another pull request.

@shakyShane
Copy link
Contributor

@shinnn - I understand your reservations - but I think this is a special case as we already allow certain options to pass through anyway.

@bitjson - I think, as you alluded to, this is a good opportunity to disconnect from handling any serve-static configuration by just passing a new hash straight through. We cannot allow a top level property however as we use that namespace for middleware/routes, but I decided upon the following for clarity.

server:  {
    baseDir: "./app",
    serveStaticOptions: {
        /* passed directly to serve-static */
    }
}

No mistake can be made here, it doesn't interfere we anything we are already doing and it can safely be introduced without breaking anything.

@shakyShane
Copy link
Contributor

@bitjson 9da345d committed in your name as I very much appreciated your contribution but could not use it directly. Either way when this lands you can just pass serveStaticOptions as mentioned above. :)

@bitjson
Copy link
Contributor Author

bitjson commented Mar 29, 2015

That looks like an excellent solution, thanks! And I appreciate the nod, thank you.

Looking forward to it landing.

@shakyShane
Copy link
Contributor

npm install [email protected]

:)

@bitjson
Copy link
Contributor Author

bitjson commented Mar 29, 2015

Impressive turnaround, thanks!

@shakyShane shakyShane reopened this Mar 29, 2015
@shakyShane shakyShane closed this Mar 29, 2015
@shakyShane
Copy link
Contributor

It was just good timing as I've been pushing to release http://quick.as/j1of99bb :)

@bitjson
Copy link
Contributor Author

bitjson commented Mar 29, 2015

Browser-sync just keeps getting better and better. Nice work!

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

Successfully merging this pull request may close these issues.

4 participants