gulpjs / vinyl

Virtual file format.
MIT License
1.28k stars 107 forks source link

Contents cannot be a `Stream` object #140

Closed demurgos closed 6 years ago

demurgos commented 6 years ago

The following code throws an error:

const Stream = require("stream").Stream;
const file = new Vinyl({contents: new Stream()});
TypeError: Cannot read property 'objectMode' of undefined
  at new Cloneable (node_modules/cloneable-readable/index.js:13:41)
  at Cloneable (node_modules/cloneable-readable/index.js:10:12)
  at File.set (index.js:183:13)
  at new File (index.js:33:17)
  at Context.<anonymous> (test/file.js:173:18)

(see the stack trace at the bottom of this Travis Job)

The error is caused in cloneable-readable when the stream is cloned because it is expected to be a readable stream (with a _readableState property):

function Cloneable (stream, opts) {
  if (!(this instanceof Cloneable)) {
    return new Cloneable(stream, opts)
  }

  var objectMode = stream._readableState.objectMode
  this._original = stream
  this._clonesCount = 1

  opts = opts || {}
  opts.objectMode = objectMode

  Ctor.call(this, opts)

  forwardDestroy(stream, this)

  this.on('newListener', onData)
}

The documentation is only requiring stream contents to only inherit from Stream:

The contents of the file. If options.contents is a Stream, it is wrapped in a cloneable-readable stream.

Either the documentation should have stronger requirements for streams (readable stream) or the implementation should check that the stream is readable before cloning it. There's also https://github.com/gulpjs/vinyl/issues/133 but it would require a semver major breaking change.

This error was present in older versions of vinyl and was found while migrating away from gulp-util in gulp-cssnano.

phated commented 6 years ago

Good catch! It definitely needs to be a ReadableStream. require('stream').Stream is a super old legacy API that points to a version of streams before Readable/Writable/Duplex. The readable-stream package actually does things a little differently by exporting a ReadableStream as the main export.