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

gulp.src should be a passthrough stream #396

Closed
yocontra opened this issue Apr 4, 2014 · 41 comments
Closed

gulp.src should be a passthrough stream #396

yocontra opened this issue Apr 4, 2014 · 41 comments
Assignees

Comments

@yocontra
Copy link
Member

yocontra commented Apr 4, 2014

This will alleviate the overusage of gulp-if

Example:

gulp.src('files/*.coffee')
  .pipe(coffee())
  .pipe(gulp.src('files/*.js'))
  .pipe(minify())
  .pipe(gulp.dest('dist'))
@yocontra yocontra added this to the gulp 4 milestone Apr 4, 2014
@yocontra yocontra self-assigned this Apr 4, 2014
@laurelnaiad
Copy link
Contributor

How is that to be different from leaving out that line?

@dashed
Copy link
Contributor

dashed commented Apr 4, 2014

+1.

I think it's been brought up at #290, which is then moved to gulpjs/vinyl-fs#9?

@yocontra
Copy link
Member Author

yocontra commented Apr 4, 2014

@stu-salsbury It allows you to add in more files at any point in the pipeline so you don't have to create two pipelines.

@laurelnaiad
Copy link
Contributor

Thanks, the example confused me because the files you're adding happened to also match the files that are coming from the stream that coffee() produces, but I get that you're actually grabbing the files in that directory that already are on disk and match the pattern.

@srcspider
Copy link

@contra your example is confusing, you should have it as addsrc or a similar variant,

gulp.src('files/*.coffee')         // out: .coffee files
  .pipe(coffee())                  // in: .coffee, out: .js files
  .pipe(gulp.addsrc('files/*.js')) // in: .js files, out: more .js files
  .pipe(minify())
  .pipe(gulp.dest('dist'))

...if the intention is to actually add to the current sources.

Two reasons for this:

  • gulp.src already does "something," no need for it to do more things or retcon it into rocket science
  • using gulp.src there would make it really confusing for anyone who doesn't understand streams

Even if you can't write it, everyone should be able to read it with out getting the wrong idea.

@dashed
Copy link
Contributor

dashed commented Apr 7, 2014

@srcspider There's nothing in gulp that tracks the source files -- so there's nothing to add globs to. gulp.src emits files as vinyl objects once, and only once.

@yocontra
Copy link
Member Author

yocontra commented Apr 7, 2014

@srcspider If it is confusing we can split it into a new thing

@darsain
Copy link
Contributor

darsain commented Apr 7, 2014

@contra I wouldn't say it is confusing. In fact, that's the first thing I've tried before using streamqueue to achieve the same thing after I realized it doesn't work.

@yocontra
Copy link
Member Author

yocontra commented Apr 7, 2014

What about an option to gulp.src which enables the passthrough behavior?

gulp.src('files/*.js', {add: true})

or something. Otherwise the files will override whats currently in the stream.

@darsain
Copy link
Contributor

darsain commented Apr 8, 2014

Just make gulp.src passthrough as you wanted in the first place. Why complicate things? :) I doubt that I'd ever want gulp.src to override files if I'd be including it later in pipe. That would make the whole pipe before that pointless.

If it's confusing for someone, let them declare aliases at the top of their gulpfiles:

var src = gulp.src;
var addsrc = gulp.src;

@phated
Copy link
Member

phated commented Apr 9, 2014

👍 on through stream with no config option.

@srcspider
Copy link

Darsain you miss my point.

When it comes to build processes one person will write it, which may
involve copypasting bits or an entire gulp template, and 10 or more will
rely on it and potentially know no more then npm install and gulp. Its not
like gulp, much like grunt, implies knowing node, or can only be used for
node based projects.

My issue with it is that currently gulp has a very weak barrier of entry,
requiring only very simple javascript thats so basic it might as well be
pseudocode. Using src twice there however as obvious from the argument
brought forth so far requires node streams lingo to even explain why it
would work. Its much better to keep the api dumb and self documenting;
having it as a seperate method keeps its uses easy to understand, readable
and not dependent on understanding its implementation.

Of course i say this in the context of current gulp; somehow i see the
whole bach, registry, and whatever else is requirer to get error reporting
and source maps properly supported, messing with the current nice gulp
sybtax to the point where what i said might not mater anymore.
On Apr 8, 2014 9:45 AM, "darsain" notifications@github.com wrote:

Just make gulp.src passthrough as you wanted in the first place. Why
complicate things? :) I doubt that I'd ever want gulp.src to override files
if I'd be including it later in pipe. That would make the whole pipe before
that completely pointless.

If it's confusing for someone, let them declare aliases at the top of
their gulpfiles:

var src = gulp.src;var addsrc = gulp.src;

Reply to this email directly or view it on GitHubhttps://github.com//issues/396#issuecomment-39820139
.

@phated
Copy link
Member

phated commented Apr 9, 2014

@srcspider I disagree, the bach changes will make it more straight forward. Gulp is easy because it is just node, so sticking with custom behavior, like through streams, wouldn't add any complexity that streams don't already have.

@srcspider
Copy link

I never said it would make it worse, I merely implied it might make it less
ubiquitous based on what I've seen of bach. And no I'm obviously not
thinking node gurus when I say that.

I dont understand why my little opinion is so hard to get. Most people wont
care to read the docs on node pipe or streams before using gulp. They
expect gulp to be just a more advanced modern make, ie. minimum programming
knowhow required, super easy to use.

Even when someone who understands how node pauses and resumes streams uses
gulp the case of explaining to someone who doesnt is not uncommon enough,
from my experience with grunt. Implementation details intersecting with the
explanation of what something does is just a pain in the ass.

Imagine if to make someone understand what the pipe character does in a
terminal you had to recite linux kernel implementation gospel.
On Apr 9, 2014 4:32 PM, "Blaine Bublitz" notifications@github.com wrote:

@srcspider https://github.com/srcspider I disagree, the bach changes
will make it more straight forward. Gulp is easy because it is just node,
so sticking with custom behavior, like through streams, wouldn't add any
complexity that streams don't already have.

Reply to this email directly or view it on GitHubhttps://github.com//issues/396#issuecomment-39963680
.

@aeharding
Copy link

IMHO, gulp.addsrc seems unnecessary. No reason to over complicate things.

Allowing gulp.src in the middle of the stream seems intuitive to me. You're just sourcing more files to add to the stream.

In a perfect world, it might be cool to have gulp.add instead of gulp.src (and maybe gulp.remove later...)

@laurelnaiad
Copy link
Contributor

It's a breaking change for people who have been using it to switch contexts.... how would someone go about replacing one set of files with a new set once gulp.src is additive rather than replacing? Is that a job for a plugin or gulp?

@aeharding
Copy link

@stu-salsbury Is that usage even documented? Why do that instead of breaking it out into multiple separate streams/tasks? Seems like a case where breaking that behavior would be good.

@laurelnaiad
Copy link
Contributor

@contra mentioned it in a stackoverflow post, and to me it seems logical that that is the behavior if no other behavior is explicitly documented. I'm not attached to it, though I've used it. In fact, I just used it in an example on stackoverflow a coupe of days ago: http://stackoverflow.com/questions/22563539/gulp-passing-parameters-to-task-from-watch-declaration/22952952#22952952 . The use case is essentially that there's one file that triggers a watch, and I want to do some stuff with that, and then add all of the other relevant files to the stream. So in that example, if the behavior changed, I'd have two instances of the file in my stream.

If gulp.src reconciles/removes dupes (using path.resolve) then I think this particular use case would be met, but of course that means that it would have to cache the contents as it goes and do comparisons along the way, which would be heavy.

I think there is a need for more stream-contents management functionality... it's easy enough to remove files from the stream or modify files along the way, but there's no real way to dedupe (or better yet, not introduce dupes in the first place). It's that much worse if the files are actually being read from disk when they're being added to the stream.

Making it necessary to split it into multiple tasks breaks the guidance that you don't string tasks together, you string processors (inbuilt or plugins) together. Requiring people to handle this in gulpfile land seems onerous, because they need to go outside the pattern of straight processor chains to manage the timing... and it's another one of those cases in my mind where there's a multiple-stream situation that may be too complex to expose to gulpfile authors and better solved with the right plugins or core functionality.

@yocontra
Copy link
Member Author

@stu-salsbury dedupe is easy and not a big deal at all

@laurelnaiad
Copy link
Contributor

For gulp.src, "dontdupe" (as an automatic feature) would be even better for speed purposes, and especially critical if the incoming stream has modified files in the buffers! I imagine that's your plan, though :)

@leefernandes
Copy link

+1 gulp.src as a passthrough

@yocontra
Copy link
Member Author

Closing this, not a priority for gulp 4 and we have probably decided against it overall since it becomes confusing

@jdan
Copy link

jdan commented Apr 21, 2014

Is there a current workflow for adding files to the stream midway? I need to conditionally add files based on the content of a previous file.

@Aaronius
Copy link

Aaronius commented May 7, 2014

+1 gulp.src as a passthrough

@laurelnaiad
Copy link
Contributor

@jdan you can use event-stream's merge or various other stream handlers that can do ordered merging and the like to accomplish this. I use this all over the place in combination with gulp-filter and an offshoot of gulp-clone I wrote to branch files into multiple streams (mainly so I can output files to more than one directory in one task, with different actions taking place on the different branches -- e.g. I build a file one way for the "build" target, and another way for the "unit" target where I run unit tests on it). I merge my branches together at the end so I (a) can write them all with one dest stream and (b) so I know when all of the task's work is complete, since the branches are re-merged and the ultimate resulting stream of the task only ends when they have all ended.

@urish
Copy link

urish commented May 13, 2014

For those who are interested in a passthrough version of gulp.src, I created a gulp-add-src plugin that does this. Your feedback will be appreciated :-)

@yocontra
Copy link
Member Author

@urish You should rely on vinyl-fs, not gulp, for a smaller dependency tree. gulp's .src fn just points to that.

@yocontra
Copy link
Member Author

Also just to be clear, I have decided against doing this in gulp.src but I still think this should exist. It belongs in a new API though, like gulp.add, not cramming it into an existing one.

@urish
Copy link

urish commented May 14, 2014

@contra Good point about vinyl-fs, released new 0.1.0 version which relies on vinyl-fs in place of gulp. Thanks!

@yocontra
Copy link
Member Author

FYI gulp.src is a passthrough in 3.8 (vinyl 0.3)

@wbyoung
Copy link

wbyoung commented Jun 27, 2014

@contra it seems this is supposed to work now, but I'm having the same problem as others. It's not appending, but simply replacing the files in the stream with the new source.

@yocontra
Copy link
Member Author

gulpjs/vinyl-fs#25

@wbyoung
Copy link

wbyoung commented Jun 27, 2014

@contra thanks. I've subscribed to that one.

@corydorning
Copy link

update gulp to use vinyl fs 0.3.9, which apparently fixes this issue:
gulpjs/vinyl-fs#25

@balthazar
Copy link

This is not fixed yet.

@jednano
Copy link
Contributor

jednano commented Oct 14, 2014

I think this would be less confusing if the code example also showed a before example, using gulp-if.

@chandru9279
Copy link

The last gulp-src wins. Tried today.

gulp.src is not a pass-through stream yet..

@Janpot
Copy link

Janpot commented Oct 15, 2014

As far as I understand stream1.pipe(stream2) always returns stream2 so it's very logical to me that you end up with the last gulp-src. I see this notion of 'passthrough stream' being tossed around here, I have a feeling it's not well understood here what a passthrough stream does exactly.

@chandru9279
Copy link

So if I wanted to source some more files in the middle of the pipeline, preserving whats already in the pipeline is there a way to do it?

@Janpot
Copy link

Janpot commented Oct 15, 2014

I usually use merge() from https://github.com/dominictarr/event-stream#merge-stream1streamn

return es.merge(
  gulp.src('./javascript/*.js')
    .pipe(minify()),
  gulp.src('./already-minified/*.js')
)
  .pipe(concat())
  .pipe(gulp.dest('...'));

@chandru9279
Copy link

Thanks, I ended up using streamqueue

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

No branches or pull requests