ExpressJS easily serve static files

Question

I have a NodeJS package, staticize, which takes an object of HTTP routes -> file locations, then serves them dynamically on each request.

Could I get some feedback on the latest rewrite of the main file? (Also available on GitHub here)

var fs = require('fs');
var mime = require('mime');
module.exports = function(app, routes) {
 // loop through routes
 for (var index in routes) {
 // closure: route is the HTTP request route, path is the file path
 (function (route, path) {
 app.get(route, function(req, res) {
 // create a readable stream
 var stream = fs.createReadStream(path, {encoding: 'utf8'});
 stream
 .once('error', function() {
 res.status(500).end('Server error.');
 console.error('ERROR (staticize): Could not open readable stream to ' + path);
 })
 .once('readable', function() {
 // create headers
 var headers = {
 'Content-Type': mime.lookup(path)
 };
 // write head then pipe response
 res.writeHead(200, headers);
 stream.pipe(res);
 });
 });
 })(index, routes[index]);
 }
};

It is called simply like this:

staticize(app, {
 '/foo': '../test/foo/bar.txt',
 '/foo/more': '../test/foo-extra/bar.txt'
});

I've tried to perfect it as much as possible, but I may have overlooked some things.

• netpoetica: I'm a bit confused by "for index in routes" - shouldn't it be "for route in routes"?

Answer 1

It looks pretty good and definitely works, but I would approach this a little bit differently and try to avoid that closure - it's a little bit difficult to read and it's also unnecessary. You can use Object.keys if you want to be able to use an object for your routes parameter.

module.exports = function(app, routes) {
 // loop through routes
 var len = routes.length,
 keys = Object.keys(routes),
 // Define path and stream outside of the loop, at the top
 // of the module as private vars and reuse (they are
 // hoisted by JavaScript anyway.
 url,
 path,
 stream;
 // Use a while loop here because it doesn't matter which
 // order you setup these routes in. This is the least verbose.
 while(len--){
 url = keys[len];
 app.get(url, function(req, res) {
 path = routes[url];
 // create a readable stream
 stream = fs.createReadStream(path, {encoding: 'utf8'});
 stream
 .once('error', function() {
 res.status(500).end('Server error.');
 console.error('ERROR (staticize): Could not open readable stream to ' + path);
 })
 .once('readable', function() {
 // create headers
 var headers = {
 'Content-Type': mime.lookup(path)
 };
 // write head then pipe response
 res.writeHead(200, headers);
 stream.pipe(res);
 });
 }); 
 }
};

Personally, I would use an array with named properties instead of an Object for my routes, but key/value is OK. Something like this would allow you to just do a routes.forEach() and have direct access without having to use Object.keys:

staticize(app, [
 {
 url: '/foo',
 path: '../test/foo/bar.txt'
 }, {
 url: '/foo/more',
 path: '../test/foo-extra/bar.txt'
 }
]);
module.exports = function(app, routes) {
 // Use a forEach here instead of a loop
 routes.forEach(function(route){
 app.get(route.url, function(req, res) {
 // Save the route since we use it three times in here
 var path = route.path;
 // create a readable stream
 var stream = fs.createReadStream(path, {encoding: 'utf8'});
 stream
 .once('error', function() {
 res.status(500).end('Server error.');
 console.error('ERROR (staticize): Could not open readable stream to ' + path);
 })
 .once('readable', function() {
 // create headers
 var headers = {
 'Content-Type': mime.lookup(path)
 };
 // write head then pipe response
 res.writeHead(200, headers);
 stream.pipe(res);
 });
 }); 
 });
};

• Brendan: Thanks for the advice! I'll definitely implement some of the changes.