teambition / merge2

Merge multiple streams into one stream in sequence or parallel (~119M/month downloads).
MIT License
170 stars 14 forks source link

As of gulp 4, merged streams have 0 files if they weren't piped #19

Closed tandrewnichols closed 6 years ago

tandrewnichols commented 6 years ago

For example, this doesn't work.

const debug = require('gulp-debug');
const merge = require('merge2');
const gulp = require('gulp');

return merge(
  gulp.src('file/a.js'),
  gulp.src('file/b.js')
).pipe(debug({ title: 'Files:' })) // outputs "Files: 0"

But this does.

const debug = require('gulp-debug');
const merge = require('merge2');
const gulp = require('gulp');

return merge(
  gulp.src('file/a.js').pipe(debug({ title: 'Files in a:' })), // outputs "Files in a: 1"
  gulp.src('file/b.js').pipe(debug({ title: 'Files in b:' })) // outputs "Files in b: 1"
).pipe(debug({ title: 'Files:' })) // outputs "Files: 0" // outputs "Files: 2"

I noticed this when I switched from the alpha version of gulp 4 to the one installed via gulp@next (which is 4.0.0).

zensh commented 6 years ago

Hi, this bug comes from `gulp4 -> vinyl-fs -> to-through: https://github.com/gulpjs/to-through/blob/master/index.js#L39

tandrewnichols commented 6 years ago

I don't understand streams well enough to know what that comment means. Are you saying it's a bug in to-through? Or just highlighting something that changed that's affecting this lib?

tandrewnichols commented 6 years ago

To add to the weirdness . . .

const noop =  through.obj((file, enc, cb) => cb(null, file));

return merge(
  gulp.src([/*list of 8 files */]).pipe(noop),
  gulp.src([/*list of 32 files */]).pipe(noop)
).pipe(debug({ title: 'Files:' })) // Outputs "Files: 15"

which I don't understand cause that's basically exactly what gulp-debug does, but using gulp-debug after each .src() makes the last debug correctly see 40 files.

tandrewnichols commented 6 years ago

Ok, returning a through function makes it work correctly (as opposed to using through directly. So ignore that second item. The initial issue still exists, however.

zensh commented 6 years ago

Fixed, v1.2.1. The finish event will emit before data event in "to-through". But "merge2" should not listen finish, because it is a writeable stream event.

tandrewnichols commented 6 years ago

Fantastic! I'll give it a try!