ForbesLindesay / browserify-middleware

express middleware for browserify, done right
http://browserify.org
MIT License
381 stars 66 forks source link

Precompile option #33

Closed bjoerge closed 10 years ago

bjoerge commented 10 years ago

Here's a suggested fix for #31 / #32

Introduced a precompile setting that can be set according to the version (directory, file or modules):

This is how to use it:

// Precompile a single file in a directory
app.use('/js', browserify('./client/dir', {
  cache: true,
  precompile: 'beep.js'
}));

// Precompile multiple files in a directory
app.use('/js', browserify('./client/dir', {
  cache: true,
  precompile: ["beep.js", "subdir/boop.js"]
}));

// Precompile a browserified file at a path
app.get('/js/file.js', browserify('./client/file.js', {
  cache: true,
  precompile: true
}));

// Precompile a bundle exposing `require` for a few npm packages.
app.get('/js/bundle.js', browserify(['hyperquest', 'concat-stream'], {
  cache: true,
  precompile: true
}));

A few things to consider:

Any suggestions/improvements are welcome :-)

ForbesLindesay commented 10 years ago

The diff seems to be a bit messed up, which is making this tricky to review. I also think it looks like you've got your arrays vs. strings mixed up a little. modules is an array, so won't work if you pass it to path.resolve. Also, all paths should already be resolved before they get to the precompile function since they get resolved in index.js. Another thing I noted is that the documentation seems to suggest you can pass a string when using it for a folder but the folder code seems to assume it's an array.

bjoerge commented 10 years ago

Yeah, line endings got messed up. Sorry about that. I've fixed it now.

About string vs array in the configuration option: Since options.precompile is passed through arrayify in settings.normalize, (like settings.noParse, etc), it should work both with strings and arrays.

It will also work with booleans for modules and files since using precompile: true will be normalized to [true] (which is just checked for truthyness later on). At the same time, passing false to arrayify will be normalized to false, so that will work too. This may be a little bit too coincidental so I'm fine with changing it into something more clear & obvious.

Removed call to path.resolve, thanks for pointing out !

ForbesLindesay commented 10 years ago

Closing this in favour of #44