Issues with libraries using an outdated vinyl version

[asgoth]

I don't really know if it belongs here, but I'm getting errors when upgrading to vinyl-fs version 3.x.

Several libraries (the ones are found are gulp-watch and vinyl-source-stream) are using an outdated vinyl version, which causes errors using vfs.dest.

See

floatdrop/gulp-watch#295
hughsk/vinyl-source-stream#23

For now I've solved it internally by wrapping the outdated vinyl object in a new one:

import through = require('through2');
import File = require('vinyl');

/**
 * gulp-watch/vinyl-source-stream uses an outdated vinyl, which doesn't have some functions which vinyl-fs expects.
 * Can be removed when gulp-watch updates it to vinyl 2.x.
 * @returns {NodeJS.ReadWriteStream}
 */
export function upgrade(): NodeJS.ReadWriteStream {
  return through.obj(function(file: any, encoding: string, cb: through.TransformCallback): void {
    const upgradedFile = new File(file);
    cb(null, upgradedFile);
  });
}

[phated]

@asgoth yeah, vinyl-fs 3 is a major, breaking version. Plugins will need to be updated to use a version of Vinyl that supports isVinyl()

[phated]

/cc @stevelacy

[phated]

@erikkemperman do you think we should marshall vinyls from <2.x to vinyl 2+ objects? It might solve this but I'm not sure what issues might be caused by doing that.

[erikkemperman]

I’m not sure, but it feels a bit like a patch in the wrong place? Curing the symptom but not the disease. Might delay plugin adoption of vinyl 2 even, hiding this issue from sight.

But we could try first nudging the plugin devs, maybe submit a PR here and there? I’d guess the changes needed are not that hard, and probably similar from one plugin to the next.

So once we have a few over, we could then just point to examples of what to do. I might have some time to try, would gulp-watch be a good candidate?

On the other hand, if new plugin releases are not forthcoming, and this is spoiling people’s first taste of the new vinyl-fs — perhaps a quick stopgap is in order...

[phated]

@erikkemperman I've sent PRs to gulp-watch and vinyl-source-stream but haven't heard back yet. The biggest problem seems to be gulp-util consumers (which is soft-deprecated currently)

[phated]

Side note: I think I'm going to be using npm-deprecate against gulp-util today.

[erikkemperman]

@phated ah, hadn’t noticed those PRs. Might be a bit pushy if I second those, you think?

[phated]

Pushy is good

[dozer75]

Reported issue on gulp-typescript ivogabe/gulp-typescript#551 too. Seems that this is a serious breaking change :|

[phated]

There's still a ton of work to do for this but I've just published 3.0.1 with a very temporary workaround that marshalls old Vinyl objects to the version included by vinyl-fs.

[carlosvillademor]

@phated that would mean that you could use any version of vinyl via any plugin and it should not throw the

Received a non-Vinyl object indest()

error, right? Because it does still happen to me using version 3.0.1

I list here all the vinyl related dependencies. I'm using node 8.9.3 with npm 5.6.0

$ npm ls | grep vinyl
│ │ └─┬ [email protected]
│ │ └─┬ [email protected]
│ ├─┬ [email protected]
│ └─┬ [email protected]
│   └─┬ [email protected]
│ │ └── [email protected] deduped
│ │ └── [email protected] deduped
│ │ └── [email protected] deduped
│ │ └─┬ [email protected]
│ └─┬ [email protected]
│   ├─┬ [email protected]
│   └─┬ [email protected]
│     └── [email protected] deduped`

Thanks.

[erikkemperman]

The patch introduced in 3.0.1 wraps the incoming vinyl in a newer version if it finds that the isSymbolic() method is missing:
https://github.com/gulpjs/vinyl-fs/blob/master/lib/dest/prepare.js#L19

That method was added in vinyl 2, and therefore this test will detect versions older than 2.
gulpjs/vinyl@2357391

However, the error you quote is emitted even before we get there, namely here:
https://github.com/gulpjs/vinyl-fs/blob/master/lib/dest/prepare.js#L16

That's testing whether the incoming object is a vinyl at all, but it does so by checking a property which really old versions of vinyl lack:
gulpjs/vinyl@2e25ad8

So basically if the incoming vinyl is version < 0.5.3 it won't even get to the point where we try to wrap it. From your comment (I edited it for legible tree) it looks like you're getting 0.4.6 via vinyl-source-stream.

Fortunately there is a new release of vinyl-source-stream, 2.0.0, which has the latest vinyl, so that should probably be your fix.

@phated So it looks like the situation here is that we can now wrap most old vinyls but not all of them (or rather we refuse because we don't get past the isVinyl() test in dest/prepare). Is it intentional to refuse vinyls < 0.5.3 in this way?

[carlosvillademor]

Thanks @erikkemperman, upgrading to vinyl-source-stream to 2.0.0 has fixed the issue.

[phated]

I want to note that you must have vinyl-buffer version 1.0.1 when upgrading to vinyl-source-stream 2.0.0 because it required a patch to make everything work. (It looks like @carlosvillademor already had that version).

So it looks like the situation here is that we can now wrap most old vinyls but not all of them (or rather we refuse because we don't get past the isVinyl() test in dest/prepare). Is it intentional to refuse vinyls < 0.5.3 in this way?

@erikkemperman yes, it was intentional because those versions are missing critical parts to the ecosystem. Vinyl 0.5.3 came with gulp-util, so that's what most people would be using. I might have been able to slip a 0.4.6 -> 0.5.3 change into vinyl-source-stream but I want people to upgrade to 2.0.0 so I don't really care to do that now.