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.

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

Convert a chunk to a string. #7

Closed
wants to merge 3 commits into from
Closed

Convert a chunk to a string. #7

wants to merge 3 commits into from

Conversation

rickihastings
Copy link

In latest node versions it's possible that chunk is an object when using this module with gulp streams as vinyl files return objects. Because of this the module breaks, as a sanity check we should be converting the chunk to a string to ensure a buffer can be created from it.

This issue can be replicated by creating a gulp task which creates an array of promises converted from gulp streams. This appears to be broken in node 5.x, not sure when it became an issue though. Our build process previously ran on 0.13.x

var sources = ['./folder/*.js', './folder/*.css']; 

gulp.task('test', function (done) {
  var promises = sources.map(function (source) {
    return streamToPromise(gulp.src([source]));
  });

  Promise.all(promises).then(function () {
    done();
  });
});

In latest node versions it's possible that chunk is an object when using this module with gulp streams as vinyl files return objects. Because of this the module breaks, as a sanity check we should be converting the chunk to a string to ensure a buffer can be created from it.
@bendrucker
Copy link
Owner

Not sure this is really correct. In your case you just want the object chunks as strings but I'm not sure that's universal. I think the only correct way to concat object stream chunks is as an array.

@rickihastings
Copy link
Author

That's actually broken the tests and yielded a different result. Ignore that last commit, I'll fix this soon. In my usages - that change actually worked fine.

Is it worth maybe checking if the type of a chunk is an object and creating a buffer from an array, else creating a buffer from the chunk as normal?

@bendrucker
Copy link
Owner

Is it worth maybe checking if the type of a chunk is an object and creating a buffer from an array, else creating a buffer from the chunk as normal?

Actually just a straight up array would be correct in that case

…k is an object

I can't pass an array in without creating a buffer from it because Buffer.concat uses Buffer.copy internally. Fixed the tests.
@derhuerst
Copy link

I'd like to use this module, but it does not support streams in object mode as I'd expect. Why not just concat them into an array? stream-sink does it the same way.

@bendrucker
Copy link
Owner

@derhuerst Sorry for the delay, you're correct that resolving an array would be expected. I've added that and am closing this PR out since it conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants