grncdr / merge-stream

Merge multiple streams into one interleaved stream
MIT License
214 stars 16 forks source link

End is never invoked when no streams are added #26

Closed rbayliss closed 7 years ago

rbayliss commented 7 years ago

Hey! If I have some code that creates a merged stream, then dynamically adds a bunch of streams, I have to specially handle the case where no streams were added. Ex:

var stream = merge(config.map(function(pack) {
    return gulp.src(pack.src)...
}));
return stream.isEmpty() ? null : stream;

If I fail to do this, the stream never emits the end event. I realize this is kind of a tricky edge case, since you can never assume when a user is done adding streams, but how about adding a finalize() method or something similar that calls done() immediately if the pool is empty?

stevemao commented 7 years ago

If I fail to do this, the stream never emits the end event.

Sorry, I'm not exactly sure what you mean by this. If you fail to do what? It would be great if you could provide a more concrete use case. Thanks :)

rbayliss commented 7 years ago

Sure, yeah. Here is a more concrete example:

var mergeStream = require('merge-stream');
...
gulp.task('build:scss', 'Build SCSS files', function () {
    var streams = mergeStream();
    config.scss.forEach(function (pack) {
      var stream = gulp
        .src(pack.src)
        .pipe(sourcemaps.init())
        .pipe(sass(pack.sassOptions).on('error', sass.logError))
        .pipe(autoprefixer(pack.prefix))
        .pipe(csso(pack.csso))
        .pipe(sourcemaps.write(pack.maps))
        .pipe(gulp.dest(pack.dest));
      streams.add(stream);
    });

    return streams;
  });

If config.scss is an empty array, then streams never emits the end event, since no streams get added to it. So that's the issue. What I'm proposing is a way to say that we are done merging in streams, so done should be called immediately if there's nothing in the pool.

stevemao commented 7 years ago

What about

if (streams.isEmpty) streams.end()

?

rbayliss commented 7 years ago

Yeah, that works. I was trying to avoid worrying about the empty-ness of what's inside, but this covers the use case. Thanks!