browserify / factor-bundle

factor browser-pack bundles into common shared bundles
Other
402 stars 27 forks source link

factor-bundle does not close writable streams when a browserify error occurs #82

Open islemaster opened 8 years ago

islemaster commented 8 years ago

I am attempting to set up a fairly complex JavaScript pipeline using browserify, watchify, babelify, factor-bundle, and eventually envify and uglifyify. Along the way I ran into issue #61 where my process was closing early and truncating the output from one of factor-bundle's streams.

Once I realized what was going on, I started working on a way to detect when factor-bundle is done with its work. I construct my own writable streams inside a function that I pass to factor-bundle's outputs argument, attaching event handlers to the finish (or unpipe) events on those streams, making each event resolve a Promise and waiting for Promise.all() to resolve.

The whole thing looks roughly like this:

var files = ['x.js', 'y.js'];
var streamPromises = [];

var b = browserify({
  cache: {}, // Required for watchify
  packageCache: {} // Required for watchify
}).
    add(files).
    plugin('factor-bundle', {
      outputs: function () {
        return files.map(makeStream);
      }
    }).
    plugin(watchify).
    on('update', function () {
      console.log('Changes detected...');
      bundle();
    });

function bundle(onComplete) {
  streamPromises.length = 0; // Clear promises before each rebuild
  b.bundle().
      on('error', function (err) {
        console.log(err.message);
        this.emit('end'); // Allows watchify to continue
      }).
      pipe(makeStream('common.js'));
  Promise.all(streamPromises).then(onComplete);
}

function makeStream(file) {
  var _resolve;
  streamPromises.push(new Promise(function (resolve) {
    _resolve = resolve;
  }));
  return fs.createWriteStream(file).
      on('finish', function () {
        console.log('Wrote ' + file);
        _resolve();
      });
}

bundle(function () {
  console.log('All streams are done');
});

It's not pretty, but it works - as long as the build is successful.

If browserify encounters an error for some reason (say, a SyntaxError in the JS you are bundling) then factor-bundle does create the writable streams, but they never fire a finish or unpipe event, or even its own 'error' event. Granted, in most use cases this sort of error would be uncaught and would end the process, auto-closing the streams in question. For use with watchify though, I manually catch the error and call this.emit('end'); as recommended here (and I'm open to correction if this is the wrong way to handle compilation errors with watchify). When I do so, the main browserify stream fires its finish and unpipe events, but factor-bundle's streams do not.

Is this a bug? I guess I expected factor-bundle to take ownership of the output streams and guarantee that they were closed whenever its work is done or aborted. I wrote a test to that effect, as a minimum repro case. As it stands now it sounds like I need to manually close all of the streams I pass to factor-bundle if an error occurs, but I'm starting to feel like I'm reaching farther into the guts of this plugin than I'm supposed to.

Related questions:

I admit I might have totally the wrong approach here - if there's something fundamentally wrong about my approach, please correct me! If this is a real issue, I'd be happy to try and track down a solution.