Repository: gulpjs/vinyl-fs
Pull request #291 (2017-12-22 20:10:30)

Fix: Marshall old Vinyl objects to new Version in dest()
@erikkemperman @asgoth @stevelacy @doowb What do y'all think of this as the solution to #288 and gulpjs/gulp/issues/2065

doowb: @phated LGTM! I like that a new Vinyl instance is created only when an old Vinyl instance is detected.
erikkemperman: Not sure if we call `isSymbolic()` there, but if so should this be done for `symlink` and even `src` as well? And perhaps add a comment that this is to deal with out-of-date plugins and hopefully could be removed at some point?
stevelacy: I do also agree with @erikkemperman relating to also check/modify `symlink`. The rest looks good
phated: @erikkemperman @stevelacy I've added the marshalling to `.symlink` but I don't think it's necessary because we don't check `.isSymbolic()` in that API - (`.src` actually always marshalls in the `wrap-file` stream).  I've also added erik's requested TODO.

erikkemperman: @phated Looks good to me, thanks!

Patch:
diff --git a/lib/dest/prepare.js b/lib/dest/prepare.js
index 11c8f4a..b3ea3d4 100644
--- a/lib/dest/prepare.js
+++ b/lib/dest/prepare.js
@@ -16,6 +16,11 @@ function prepareWrite(folderResolver, optResolver) {
       return cb(new Error('Received a non-Vinyl object in `dest()`'));
     }
 
+    // TODO: Remove this after people upgrade vinyl/transition from gulp-util
+    if (typeof file.isSymbolic !== 'function') {
+      file = new Vinyl(file);
+    }
+
     var outFolderPath = folderResolver.resolve('outFolder', file);
     if (!outFolderPath) {
       return cb(new Error('Invalid output folder'));
diff --git a/lib/symlink/prepare.js b/lib/symlink/prepare.js
index ffc5b7d..dd2ec1a 100644
--- a/lib/symlink/prepare.js
+++ b/lib/symlink/prepare.js
@@ -16,6 +16,11 @@ function prepareSymlink(folderResolver, optResolver) {
       return cb(new Error('Received a non-Vinyl object in `symlink()`'));
     }
 
+    // TODO: Remove this after people upgrade vinyl/transition from gulp-util
+    if (typeof file.isSymbolic !== 'function') {
+      file = new Vinyl(file);
+    }
+
     var cwd = path.resolve(optResolver.resolve('cwd', file));
 
     var outFolderPath = folderResolver.resolve('outFolder', file);


Test patch:
diff --git a/test/dest.js b/test/dest.js
index 85f438e..af8dcf5 100644
--- a/test/dest.js
+++ b/test/dest.js
@@ -16,6 +16,7 @@ var applyUmask = require('./utils/apply-umask');
 var testStreams = require('./utils/test-streams');
 var always = require('./utils/always');
 var testConstants = require('./utils/test-constants');
+var breakPrototype = require('./utils/break-prototype');
 
 var from = miss.from;
 var pipe = miss.pipe;
@@ -1018,4 +1019,48 @@ describe('.dest()', function() {
       concat(assert),
     ], done);
   });
+
+  it('does not marshall a Vinyl object with isSymbolic method', function(done) {
+    var file = new File({
+      base: outputBase,
+      path: outputPath,
+    });
+
+    function assert(files) {
+      expect(files.length).toEqual(1);
+      // Avoid comparing stats because they get reflected
+      delete files[0].stat;
+      expect(files[0]).toMatch(file);
+      expect(files[0]).toBe(file);
+    }
+
+    pipe([
+      from.obj([file]),
+      vfs.dest(outputBase),
+      concat(assert),
+    ], done);
+  });
+
+  it('marshalls a Vinyl object without isSymbolic to a newer Vinyl', function(done) {
+    var file = new File({
+      base: outputBase,
+      path: outputPath,
+    });
+
+    breakPrototype(file);
+
+    function assert(files) {
+      expect(files.length).toEqual(1);
+      // Avoid comparing stats because they get reflected
+      delete files[0].stat;
+      expect(files[0]).toMatch(file);
+      expect(files[0]).toNotBe(file);
+    }
+
+    pipe([
+      from.obj([file]),
+      vfs.dest(outputBase),
+      concat(assert),
+    ], done);
+  });
 });
diff --git a/test/symlink.js b/test/symlink.js
index dc72861..606d92d 100644
--- a/test/symlink.js
+++ b/test/symlink.js
@@ -14,6 +14,7 @@ var isWindows = require('./utils/is-windows');
 var testStreams = require('./utils/test-streams');
 var always = require('./utils/always');
 var testConstants = require('./utils/test-constants');
+var breakPrototype = require('./utils/break-prototype');
 
 var from = miss.from;
 var pipe = miss.pipe;
@@ -940,4 +941,50 @@ describe('symlink stream', function() {
       concat(assert),
     ], done);
   });
+
+  it('does not marshall a Vinyl object with isSymbolic method', function(done) {
+    var file = new File({
+      base: outputBase,
+      path: outputPath,
+    });
+
+    function assert(files) {
+      expect(files.length).toEqual(1);
+      // Avoid comparing stats because they get reflected
+      delete files[0].stat;
+      expect(files[0]).toMatch(file);
+      expect(files[0]).toBe(file);
+    }
+
+    pipe([
+      from.obj([file]),
+      vfs.symlink(outputBase),
+      concat(assert),
+    ], done);
+  });
+
+  it('marshalls a Vinyl object without isSymbolic to a newer Vinyl', function(done) {
+    var file = new File({
+      base: outputBase,
+      path: outputPath,
+      // Pre-set this because it is set by symlink
+      symlink: outputPath,
+    });
+
+    breakPrototype(file);
+
+    function assert(files) {
+      expect(files.length).toEqual(1);
+      // Avoid comparing stats because they get reflected
+      delete files[0].stat;
+      expect(files[0]).toMatch(file);
+      expect(files[0]).toNotBe(file);
+    }
+
+    pipe([
+      from.obj([file]),
+      vfs.symlink(outputBase),
+      concat(assert),
+    ], done);
+  });
 });
diff --git a/test/utils/break-prototype.js b/test/utils/break-prototype.js
new file mode 100644
index 0000000..9ced60e
--- /dev/null
+++ b/test/utils/break-prototype.js
@@ -0,0 +1,23 @@
+'use strict';
+
+var File = require('vinyl');
+
+function breakPrototype(file) {
+  // Set up a broken prototype
+  var oldProto = {};
+  Object.getOwnPropertyNames(File.prototype).forEach(function(key) {
+    if (key !== 'isSymbolic') {
+      var desc = Object.getOwnPropertyDescriptor(File.prototype, key);
+      Object.defineProperty(oldProto, key, desc);
+    }
+  });
+
+  // Assign the broken prototype to our instance
+  if (typeof Object.setPrototypeOf === 'function') {
+    Object.setPrototypeOf(file, oldProto);
+  } else {
+    file.__proto__ = oldProto;
+  }
+}
+
+module.exports = breakPrototype;