Repository: gulpjs/vinyl-fs
Pull request #154 (2016-02-06 22:41:16)

Don't pass `read` option from `src` to `through2`
As promised in [#153](https://github.com/gulpjs/vinyl-fs/issues/153)

phated: @erikkemperman can you add a test for this?  I'm not sure the behavior passing it caused
erikkemperman: I'm not sure what it would test...? What I noticed is if I pass a function to the `read` option of `src` -- expecting its boolean return value to be used to determine whether or not to read the file contents -- it was instead being treated as the `read` implementation of stream, because it is passed on to `through2`.

So I suppose I could write a test that does that and check that the function is not called (that my PR makes sure it's not passed to through2). But you mentioned that function values are going to be supported soon, so then this test would be in the way.

Anyway, the diff is pretty self-evidently doing what it advertises, I thought?
phated: Yeah, I just would like some sort of test so I don't add a regression during a refactor. I actually tried refactoring my other PR today but all the tests caught my mistakes and I realized I couldn't do what I was trying. I'd be happy with the test suggestion you made, it shouldn't be in the way. 
erikkemperman: All right, will do. I'll add a comment though because I do expect this test might confuse whoever ends up implementing function-value support for options like `read` and `buffer`.

Patch:
diff --git a/lib/fileOperations.js b/lib/fileOperations.js
index e323fb7..4c194d4 100644
--- a/lib/fileOperations.js
+++ b/lib/fileOperations.js
@@ -3,6 +3,7 @@
 var fs = require('graceful-fs');
 var assign = require('object-assign');
 var isEqual = require('lodash.isequal');
+var isBuffer = require('is-buffer');
 var isValidDate = require('vali-date');
 
 // TODO shared module
@@ -167,7 +168,7 @@ function writeFile(path, data, options, callback) {
     options = {};
   }
 
-  if (!Buffer.isBuffer(data)) {
+  if (!isBuffer(data)) {
     callback(new TypeError('Data must be a Buffer'));
     return;
   }
diff --git a/lib/src/index.js b/lib/src/index.js
index a20f705..c2d0463 100644
--- a/lib/src/index.js
+++ b/lib/src/index.js
@@ -43,6 +43,10 @@ function src(glob, opt) {
     followSymlinks: true,
   }, opt);
 
+  // Don't pass `read` option on to through2
+  var read = options.read !== false;
+  options.read = undefined;
+
   var inputPass;
 
   if (!isValidGlob(glob)) {
@@ -54,20 +58,20 @@ function src(glob, opt) {
   var outputStream = globStream
     .pipe(normalizePath(options))
     .pipe(resolveSymlinks(options))
-    .pipe(through2.obj(opt, createFile));
+    .pipe(through2.obj(options, createFile));
 
   if (options.since != null) {
     outputStream = outputStream
       .pipe(filterSince(options.since));
   }
 
-  if (options.read !== false) {
+  if (read) {
     outputStream = outputStream
       .pipe(getContents(options));
   }
 
   if (options.passthrough === true) {
-    inputPass = through2.obj(opt);
+    inputPass = through2.obj(options);
     outputStream = duplexify.obj(inputPass, merge(outputStream, inputPass));
   }
   if (options.sourcemaps === true) {
diff --git a/package.json b/package.json
index 61bee8e..fa33a3d 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "vinyl-fs",
   "description": "Vinyl adapter for the file system",
-  "version": "2.3.3",
+  "version": "2.3.2",
   "homepage": "http://github.com/wearefractal/vinyl-fs",
   "repository": "git://github.com/wearefractal/vinyl-fs.git",
   "author": "Fractal <contact@wearefractal.com> (http://wearefractal.com/)",
@@ -15,6 +15,7 @@
     "glob-stream": "^5.2.0",
     "graceful-fs": "^4.0.0",
     "gulp-sourcemaps": "^1.5.2",
+    "is-buffer": "^1.1.2",
     "is-valid-glob": "^0.3.0",
     "lazystream": "^1.0.0",
     "lodash.isequal": "^4.0.0",


Test patch:
diff --git a/test/fileOperations.js b/test/fileOperations.js
index c5ccaac..454eaba 100644
--- a/test/fileOperations.js
+++ b/test/fileOperations.js
@@ -7,7 +7,6 @@ var fs = require('graceful-fs');
 var del = require('del');
 var path = require('path');
 var File = require('vinyl');
-var buffer = require('buffer');
 var defaultResolution = require('default-resolution');
 
 var fo = require('../lib/fileOperations');
@@ -557,7 +556,7 @@ describe('writeFile', function() {
     });
   });
 
-  it('passes an error if called with string as data', function(done) {
+  it('passes an error if called without buffer for data', function(done) {
     writeFile(filepath, 'test', function(err) {
       expect(err).toExist();
 
@@ -565,31 +564,6 @@ describe('writeFile', function() {
     });
   });
 
-  it('does not error on SlowBuffer', function(done) {
-    if (!buffer.SlowBuffer) {
-      this.skip();
-      return;
-    }
-
-    var expected = 'test';
-    var buf = new Buffer(expected);
-    var content = new buffer.SlowBuffer(4);
-    buf.copy(content, 0, 0, 4);
-
-    writeFile(filepath, content, function(err, fd) {
-      expect(err).toNotExist();
-      expect(typeof fd === 'number').toEqual(true);
-
-      fs.close(fd, function() {
-        var written = fs.readFileSync(filepath, 'utf-8');
-
-        expect(written).toEqual(expected);
-
-        done();
-      });
-    });
-  });
-
   it('does not error if options is falsey', function(done) {
     var content = new Buffer('test');
     writeFile(filepath, content, null, function(err, fd) {
diff --git a/test/src.js b/test/src.js
index 3fa2c65..d92cb9d 100644
--- a/test/src.js
+++ b/test/src.js
@@ -396,6 +396,27 @@ describe('source stream', function() {
     stream1.pipe(stream2).pipe(bufferStream);
   });
 
+  it('should not pass options.read on to through2', function(done) {
+    // Note: https://github.com/gulpjs/vinyl-fs/issues/153
+    // In future, if/when function values are supported for options like
+    // `read` and `buffered`, the expected value here will be 1.
+    var canary = 0;
+    var expected = 0;
+    var read = function() {
+      canary++;
+      return 0;
+    };
+
+    var onEnd = function() {
+      canary.should.equal(expected);
+      done();
+    };
+
+    var buffered = [];
+    var stream = vfs.src('./fixtures/test.coffee', { cwd: __dirname, read: read });
+    var bufferStream = through.obj(dataWrap(buffered.push.bind(buffered)), onEnd);
+    stream.pipe(bufferStream);
+  });
 });
 
 describe('.src() symlinks', function() {