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

Using lazypipe at the end of a stream hangs the Gulp task #14

Closed
RIAstar opened this issue Mar 27, 2015 · 14 comments
Closed

Using lazypipe at the end of a stream hangs the Gulp task #14

RIAstar opened this issue Mar 27, 2015 · 14 comments
Labels

Comments

@RIAstar
Copy link

RIAstar commented Mar 27, 2015

If you use lazypipe at the end of a Gulp stream, the task will be properly executed, but it never finishes. If you run this task standalone this is not problematic; you will just not get the message on the comand line that the task finished. However if the task is used in a series it will cause depending tasks to wait forever.

Example code:

var lazy = lazypipe()
    .pipe(uglify)
    .pipe(gulp.dest(destination));

function task() {
    return gulp
        .src(glob)
        .pip(lazy());
}

At first sight it seems related to #7, but there's actually something different going on.
I tracked the problem down to stream-combiner not firing a finish event and it seems that Gulp relies on that event.

I opened an issue on the stream-combiner GitHub page:
dominictarr/stream-combiner#13

I guess you shouldn't have to change anything to your code, but you might want to follow up on that issue, so that you can update your dependencies when the issue gets fixed.

@OverZealous
Copy link
Owner

If your code above is correct, the issue is that you have set your gulp.dest up incorrectly. You cannot pass initialized pipe functions into a lazypipe, because it instantiates them when you call it.

Your sample code, instead, should be:

var lazy = lazypipe()
    .pipe(uglify)
    .pipe(gulp.dest, destination);

If that's not the issue, then it still doesn't make much sense, because events such as finish are passed through from the originating stream. I'm using lazypipe throughout a massive gulp build system, and if there were any issue with finish events, I would have come across it at some point.

You need to make an absolute minimal case example (I recommend a very simple gulp plugin, such as gulp-debug) to test this issue.

@RIAstar
Copy link
Author

RIAstar commented Apr 3, 2015

Sorry about the botched sample code; wrote that of the top of my head as a minimal version of the actual code, hence the mistake.

Here's a fully working sample that reproduces the behaviour:

var gulp = require('gulp'),
    debug = require('gulp-debug'),
    lazypipe = require('lazypipe');

var lazydest = lazypipe()
    .pipe(debug)
    .pipe(gulp.dest, './build');

gulp.task('test', function () {
    return gulp
        .src('./src/app/**/*.*')
        .pipe(lazydest());
});

The files from ./src/app/ are copied as expected, but the test task never finishes.
Remove the .pipe(debug) line and the task finishes as expected.

As I said: the problem actually lies with stream-combiner not firing a finish event. I posted a minimal example over there as well, but using just stream-combiner itself. If anything that sample is even more minimal since it uses only through2. The owner of the repo told me to use bun instead of stream-combiner. Still need to look into that.

I should also mention that I'm running this with Gulp 4 instead of Gulp 3, but I don't see how that would change anything.

@OverZealous
Copy link
Owner

Hmm, Gulp 4 might break something, I haven't even begun looking at it (since they took so long getting there).

I'm sorry it's giving you trouble, if I get time, I'll look into bun, but I've had trouble with other stream-combiner replacements in the past, and I'm really busy with work right now.

@RIAstar
Copy link
Author

RIAstar commented Apr 4, 2015

Did some testing and it seems that there is indeed a difference between Gulp 3 and Gulp 4 and - more importantly - that it might not be related to the finish event after all.

Running the two minimal examples with Gulp 3 I get the following results:

  • stream-combiner-only case: same as with Gulp 4, meaning the combined stream doesn't fire a finish event
  • lazypipe case: works as expected

This must mean that the cause for the Gulp task not finishing is not the lack of a finish event. Back to square one :(

For reference, here's the complete stream-combiner-only case in a Gulp pipeline:

var combiner = require('stream-combiner'),
    through = require('through2');

var stream = combiner(
    through.obj(pass).on('finish', log('inner stream 1')),
    through.obj(pass).on('finish', log('inner stream 2'))
).on('finish', log('combined stream'));

function pass(file, enc, cb) {
    cb(null, file);
}

function log(id) {
    return function() {
        console.log('FINISH: ' + id);
    }
}

gulp.task('test', function () {
    return gulp
        .src('./src/app/**/*.*')
        .pipe(stream);
});

Note that I used through2 for the test because that's what gulp.dest uses too, both in Gulp 3 and Gulp 4.
Dominic suggested that stream-combiner is not really compatible with stream2, but nevertheless that doesn't seem to be the cause.

@rjferguson21
Copy link
Contributor

I switched to stream-combiner2 in the example @RIAstar posted and it seemed to properly finish the task. Based on that fact I went ahead and created a PR updated lazypipe to use stream-combiner2 and it seemed to have fixed my problem with streams composed of lazypipes in gulp not emitting finish events.

#18

Edit: After looking at it a little closer this might break things with plugins using stream.

@clintwood
Copy link

I can confirm same problem using gulp#4.0, fixed using stream-combiner2.

@ewu02
Copy link

ewu02 commented Sep 27, 2015

I'm encountering the same problem. gulp.dest() doesn't work when inside of lazypipe.

Using gulp 3.9.0

example

function writeToDevDir() {
  return lazypipe()
    .pipe(gulp.dest, './dev')();
}

gulp.task('write', function() {
  gulp.src('**/*.js')
    .pipe(writeToDevDir()); //nothing gets written
}).

@Bnaya
Copy link

Bnaya commented Dec 18, 2015

So i understand that for this moment gulp 4 and lazypipe are incompatible?

@OverZealous
Copy link
Owner

As of now, I have no intention of making lazypipe work on Gulp 4.

@goodforenergy
Copy link

For anyone looking, there are some simple workarounds listed here.

@mrDinckleman
Copy link

Is there any fixes for this issue?

@OverZealous
Copy link
Owner

No, sorry, Gulp 4 isn't even on my radar. I have no plans on supporting it, probably ever.

I get the feeling they don't care too much, either, as it's still not officially released, and it's been over four years since I first heard about the new-and-improved Gulp 4 which would include support for various features I've worked around in Gulp 3 (i.e., run-sequence).

I'd be willing to look into a PR if someone else wants to tackle it, but only one that doesn't break existing Gulp 3 functionality, or require major changes to how the code works. I've had trouble in the past with people replacing stream-combiner with another stream combining tool, that broke things in subtle ways.

Alternatively, there's nothing wrong with forking this project, or looking into one of the alternative forks. This one looks like a good option.

@strarsis
Copy link

strarsis commented Sep 25, 2021

@OverZealous: Hm, lazypipe2 seems to be already removed from GitHub. 😿

@OverZealous
Copy link
Owner

@strarsis That's too bad. It wasn't my project, so there's nothing I can do. Looks like they removed it from npm, but you can still find it if you look directly, maybe you can use that as a starting point for a fork.

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

No branches or pull requests

9 participants