Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking: Use cloneable-readable to clone streams - possibly breaking (closes #85) #99

Merged
merged 4 commits into from
Sep 19, 2016
Merged

Breaking: Use cloneable-readable to clone streams - possibly breaking (closes #85) #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6a0bfe
Breaking: Use cloneable-readable to clone streams - possibly breaking…
phated Aug 16, 2016
7d8af8c
Added test for clone stream behavior and bumped cloneable-readable to…
mcollina Sep 2, 2016
a4c3e14
Update: Add test for readable events
phated Sep 14, 2016
33a6788
Upgrade: Bumped cloneable-readable v0.4.0. (#108)
mcollina Sep 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ var isStream = require('./lib/isStream');
var isNull = require('./lib/isNull');
var inspectStream = require('./lib/inspectStream');
var normalize = require('./lib/normalize');
var Stream = require('readable-stream');
var replaceExt = require('replace-ext');
var cloneable = require('cloneable-readable');

var builtInFields = [
'_contents', '_symlink', 'contents', 'stat', 'history', 'path',
Expand Down Expand Up @@ -111,8 +111,11 @@ File.prototype.clone = function(opt) {
// Clone our file contents
var contents;
if (this.isStream()) {
contents = this.contents.pipe(new Stream.PassThrough());
this.contents = this.contents.pipe(new Stream.PassThrough());
if (typeof this.contents.clone !== 'function') {
this.contents = cloneable(this.contents);
}

contents = this.contents.clone();
} else if (this.isBuffer()) {
contents = opt.contents ? cloneBuffer(this.contents) : this.contents;
}
Expand Down Expand Up @@ -173,6 +176,11 @@ Object.defineProperty(File.prototype, 'contents', {
if (!isBuffer(val) && !isStream(val) && !isNull(val)) {
throw new Error('File.contents can only be a Buffer, a Stream, or null.');
}

if (isStream(val)) {
val = cloneable(val);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you need this? I would avoid, given you are already wrapping on demand https://github.com/gulpjs/vinyl/pull/99/files#diff-168726dbe96b3ce427e7fedce31bb0bcR85.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forget it, it's needed, I haven't looked at this for a bit of time.


this._contents = val;
},
});
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"dependencies": {
"clone": "^1.0.0",
"clone-stats": "^1.0.0",
"cloneable-readable": "^0.4.0",
"readable-stream": "^2.1.0",
"replace-ext": "^1.0.0"
},
Expand Down
92 changes: 88 additions & 4 deletions test/File.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,90 @@ describe('File', function() {
done();
});

it('should not start flowing until all clones flows', function(done) {
var contents = new Stream.PassThrough();
var options = {
cwd: '/',
base: '/test/',
path: '/test/test.coffee',
contents: contents,
};
var file = new File(options);
var file2 = file.clone();
var ends = 2;

function latch() {
if (--ends === 0) {
done();
}
}

contents.write(new Buffer('wa'));

process.nextTick(function() {
contents.write(new Buffer('dup'));
contents.end();
});

// Start flowing file2
file2.contents.on('data', function() {});

file2.contents.once('data', function() {
process.nextTick(function() {
// Starts flowing file
file.contents.on('data', function() {
ends.should.equal(2);
});
});
});

file2.contents.on('end', latch);
file.contents.on('end', latch);
});

it('should not start flowing until all clones flows', function(done) {
var contents = new Stream.PassThrough();
var options = {
cwd: '/',
base: '/test/',
path: '/test/test.coffee',
contents: contents,
};
var file = new File(options);
var file2 = file.clone();
var ends = 2;

function latch() {
if (--ends === 0) {
done();
}
}

contents.write(new Buffer('wa'));

process.nextTick(function() {
contents.write(new Buffer('dup'));
contents.end();
});

// Start flowing file2
file2.contents.on('readable', function() {
this.read();
});

file2.contents.once('readable', function() {
process.nextTick(function() {
// Starts flowing file
file.contents.on('readable', function() {
ends.should.equal(2);
});
});
});

file2.contents.on('end', latch);
file.contents.on('end', latch);
});

it('should copy all attributes over with null', function(done) {
var options = {
cwd: '/',
Expand Down Expand Up @@ -636,7 +720,7 @@ describe('File', function() {
path: '/test/test.coffee',
contents: new Stream.PassThrough(),
});
file.inspect().should.equal('<File "test.coffee" <PassThroughStream>>');
file.inspect().should.equal('<File "test.coffee" <CloneableStream>>');
done();
});

Expand All @@ -661,11 +745,11 @@ describe('File', function() {
done();
});

it('should work with Stream', function(done) {
var val = new Stream.PassThrough();
it('should wrap Stream in Cloneable', function(done) {
var val = new Stream();
var file = new File();
file.contents = val;
file.contents.should.equal(val);
(typeof file.contents.clone).should.equal('function');
done();
});

Expand Down