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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for async through transforms #26

Closed
wants to merge 3 commits into from

Conversation

junosuarez
Copy link

I tried a few ways of implementing through-like transforms which first buffer the input stream, for use with standard node-style async functions. In the end, I wound up re-implementing almost all of concat-stream in these modules.

It seems like either through functionality should be a part of concat-stream, as is implemented in this speculative PR, or else there should be a better primitive for writing concat-stream like modules.

This syntax seems clean to me, and there should be negligible overhead for creating Duplex rather than Writable streams. If you're into this approach, feel free to merge - otherwise I'll probably just publish this as another module and revisit it later to factor it better.

馃悎

@max-mapper
Copy link
Owner

@jden I am more okay with the idea of a concat-stream being a duplex stream, but I'm less sure about the optional through function. Seems like you could just as easily just use require('through') instead

@junosuarez
Copy link
Author

@maxogden so how would that work? something like this?

var fs = require('fs')
var concat = require('concat-stream')
var through = require('through2')

fs.createReadStream('foo.csv')
  .pipe(concat())
  .pipe(through(someTransformFn))
  ...

This would work perfectly well for my use case. There's some prior art for this pattern in gulp-buffer, although its readme doesn't make it immediately obvious.

@stevemao
Copy link

I think its a good idea to make this a duplex stream too.

@stevemao
Copy link

ping @jden , did you end up creating a new module?

@junosuarez
Copy link
Author

I don't think I ever did. I'm going to close this feature request issue.

@junosuarez junosuarez closed this Jun 25, 2015
@stevemao
Copy link

@jden @maxogden I think making this as a duplex stream makes sense. I have valid use cases where I would concat all streams and pipe it down.

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.

None yet

3 participants