Add an option to enable globs coming in the order they were passed in

[yocontra]

cc @lazd

[azweb76]

They are being returned in the order passed in. The first glob is being excluded because the path does not exist. Should "key" be "hey" in the following... "./fixtures/whatsgoingon/key/isaidhey/whatsgoingon/test.txt"?

[yocontra]

@azweb76 Probably. Currently they should not be coming back in the order passed in. If they are then something in node-glob is blocking

[lazd]

@azweb76, yes, that's an error in the test

[yocontra]

I corrected the test and I'm having trouble getting things to come out of order. The only explanation I can think of is that combine-stream is ordering the streams it combines but that seems unlikely. We might have to create a more complex dir structure for the "slow" glob in the test

[armed]

Is it ok, when this test fail? I've an issue with it in my current project.

it('should return a correctly ordered file name stream for three globs', function(done) {
  var globArray = [
    join(__dirname, "./fixtures/**/test.txt"),
    join(__dirname, "./fixtures/**/test.coffee"),
    join(__dirname, "./fixtures/**/test.js")
  ];
  var stream = gs.create(globArray, {cwd: __dirname});

  var files = [];
  stream.on('error', done);
  stream.on('data', function(file) {
    should.exist(file);
    should.exist(file.path);
    files.push(file);
  });
  stream.on('end', function() {
    files.length.should.equal(3);
    path.basename(files[0].path).should.equal('test.txt');
    path.basename(files[1].path).should.equal('test.coffee');
    path.basename(files[2].path).should.equal('test.js');
    done();
  });
});

[yocontra]

@armed Does that fail? If so this is a bug

[armed]

Yes, it fails. At least on my machine. I modified already existing test case: https://github.com/wearefractal/glob-stream/blob/master/test/main.js#L212

[armed]

It started failing when I changed full paths to paths with globstars.

[yocontra]

@armed Awesome I was having issues making the test fail for ordering. I'll fix them up and start working on this again

[armed]

@Contra great news! This fix is a show stopper for my current grunt -> gulp migration.

[armed]

@Contra I've been experimenting with ordering of streams. I created drop-in replacement for combine-stream which preserves ordering. It passes all tests in glob-stream master plus test which I posted above. Code may be ugly, but I've managed to build all assets in my projects without any issues.

This is what I got (maybe it will help):

var Readable = require('stream').Readable;
var util = require('util');

function OrderedStreams(options) {
  options = options || {};
  if (Array.isArray(options)) {
    options = {streams: options};
  }
  options.objectMode = true;

  Readable.call(this, options);

  var streams = options.streams || [];
  var self = this;

  if (streams.length === 0) {
    this.push(null); // no streams, close
  } else {
    // initial index in list of glob-streams
    this._currentIndex = 0;
    this._buff = {};
    this._openedStreams = streams.length;
    streams.forEach(function (s, i) {
      s.on('data', function (data) {
        // if data emmitted from stream, which is at current index
        if (i === self._currentIndex) {
          self._currentIndex++;
          self.push(data); // push data downstream
        } else {
          self._buff[i] = data; // or store in buffer for future
        }
      });
      s.on('end', function () {
        // if stream ended without any data and it at current index
        if (i === self._currentIndex) {
          self._currentIndex++; // increment index
        }
        if (!--self._openedStreams) {
          self.push(null); // close OrderedStreams
        }
      })
      s.on('error', function (e) {
        self.emit('error', e); // error event downstream
      })
    });

    this.on('finish', function() {
      streams.forEach(function (s) {
        s.end();
      });
    });
  }
}

util.inherits(OrderedStreams, Readable);

OrderedStreams.prototype._read = function () {
  // if we already have stored data push it
  var data = this._buff[this._currentIndex];
  if (data) {
    this._currentIndex++;
    this.push(data);
  }
}

module.exports = OrderedStreams;

[yocontra]

@armed got a repo so I can look over the code and the tests?

[armed]

@Contra https://github.com/armed/glob-stream, added it in root for convenience.

[yocontra]

@armed the combined-stream should be in its own package with full test coverage not included in the glob-stream package

[armed]

@Contra I wasn't planning to leave it in glob-stream, it's just experiments. If that combine-stream implementation is ok, then I create package.

[armed]

@Contra I created package for ordered stream implementation, glob-stream repo with this package is here: https://github.com/armed/glob-stream

[rschmukler]

@armed, @contra

I ran into something similar and ended up writing stream-series. You can use it like the following:

  return series(
    gulp.src(['lib/{components,pages}/**/index.js']),
    gulp.src(['lib/{components,pages}/**/*.js', '!lib/{components,pages}/**/index.js'])
  )
  .pipe(concat('build.js'))
  .pipe(gulp.dest('public/'))

[yocontra]

@rschmukler Does the latest glob-stream solve this for you? npm update should pull it in

[yocontra]

 return gulp.src(['lib/{components,pages}/**/index.js', 'lib/{components,pages}/**/*.js', '!lib/{components,pages}/**/index.js'])
  .pipe(concat('build.js'))
  .pipe(gulp.dest('public/'))

should work fine now

[rschmukler]

Ah, I am using gulp 3.4.0 which imports vinyl-fs 0.0.1, which I am guessing does not have the latest glob-stream?

I was still running into the issue but maybe I'm screwing something up...

[yocontra]

@rschmukler npm update will update the whole tree. vinyl-fs hasn't updated but it uses a semver range for glob-stream so any bug fixes to glob-stream won't require me to update the whole tree above it

[rschmukler]

Ah woops. Thanks @Contra !