Add passthrough option, fixes #25, fixes gulpjs/gulp#840
#55
Conversation
| @@ -46,7 +49,12 @@ function src(glob, opt) { | |||
| .pipe(getContents(options)); | |||
| } | |||
|
|
|||
| return outputStream.pipe(pass); | |||
| if (options.passthrough) { | |||
| var inputPass = through.obj(); | |||
There was a problem hiding this comment.
var in an if condition should fail the linter
There was a problem hiding this comment.
What linter? The project doesn't have JSCS, and JSHint has moved away from checking code style I believe. Anyway, should I move the var declarations to the top of their scope?
There was a problem hiding this comment.
@UltCombo Yeah I think you can merge inputPass and pass into the same var as well
|
How does the ordering work? Does it wait for the parent to finish, then emit? Or is it just first come first serve |
|
@contra the PR uses merge-stream, so it is first come first served -- this is gulp-add-src's default. This is also why I'm using gulp-add-src also provides methods for prepend/append, but I don't know if those are worth adding to vinyl-fs -- users that need ordered streaming can still use gulp-add-src I believe. I'm also a bit concerned that there is no deduping in their (and, consequently, our) stream merging logic. This is most likely a rare enough use case that can be addressed in the future in case such use cases actually surface. |
|
@UltCombo I think we should support some form of ordering and deduplicaton |
|
@contra which form of ordering should we prefer (or should ordering be customizable)? Not sure about deduping, it might be over-thinking it a bit too much -- I believe it is mostly a corner case, and can be cleanly solved in user code with a simple |
|
The main problem I see with supporting ordering and deduping options is that it'd be too much API pollution for unnecessary/simple abstractions. Might as well recommend people to use gulp-add-src instead of implementing |
|
|
||
| if (!isValidGlob(glob)) { | ||
| throw new Error('Invalid glob argument: ' + glob); | ||
| } | ||
| // return dead stream if empty array | ||
| if (Array.isArray(glob) && glob.length === 0) { | ||
| process.nextTick(pass.end.bind(pass)); | ||
| var pass = through.obj(); |
There was a problem hiding this comment.
@contra I don't really know what code style you want.
|
@contra I don't really see what is wrong with Anyway, I've moved the |
1 similar comment
|
@UltCombo It makes it harder to follow things imo - having vars at the top makes it easier to know what is in scope |
Add `passthrough` option, fixes #25, fixes gulpjs/gulp#840
|
Alright, no prob. |
|
Available on the gulp 4.0 branch now |
Add `passthrough` option, fixes #25, fixes gulpjs/gulp#840
Ref: gulpjs/gulp#840 #25