Silent gulp exit on duplicate tasks

[poelstra]

When a task is (e.g. accidentally) listed more than once in a 'batch' to run-sequence, gulp silently exits (with exit code 0) without running other tasks after that batch.

To reproduce, try running the following using gulp test:

var gulp = require('gulp');
var runSequence = require('run-sequence');

gulp.task('test', function(callback) {
  runSequence(['a','a'], 'b', callback);
});

gulp.task('a', function() {
    console.log('a');
});

gulp.task('b', function() {
    console.log('b');
});

This leads to the following output:

[16:52:12] Using gulpfile c:\Source\testje\gulpfile.js
[16:52:12] Starting 'test'...
[16:52:12] Starting 'a'...
a
[16:52:12] Finished 'a' after 180 μs

Whereas the following would be expected:

[16:52:59] Using gulpfile c:\Source\testje\gulpfile.js
[16:52:59] Starting 'test'...
[16:52:59] Starting 'a'...
a
[16:52:59] Finished 'a' after 174 μs
[16:52:59] Starting 'b'...
b
[16:52:59] Finished 'b' after 125 μs
[16:52:59] Finished 'test' after 3.98 ms

One way to fix this, would be to change the if (idx > -1) in the onTaskEnd() function to e.g. while ((idx = currentTaskSet.indexOf(event.task)) > -1).

[OverZealous]

Wow, that's a great find. I'm trying to decide if it makes sense to:

• Allow multiple runs (your solution—thanks for the suggestion!)
• Allow multiple runs, but print a warning
• Throw an error if a task is repeated (principle of least surprise, but eliminates the option of running a task twice)
• Ignore duplications (potentially hiding an error).

I'm actually leaning to throwing the error. If someone meant: ['clean-dev', 'clean-dist'], but accidentally typed ['clean-dev', 'clean-dev'], they might struggle to figure out why the dist directory isn't updating on the other three setups.

I can also check for it during the verification stage.

Any opinion?

[poelstra]

Yeah, throwing an error during the verification stage is probably the best solution.

One could always write a function to de-duplicate the input to run-sequence (e.g. in case of auto-generated targets), so it makes sense to stay on the safe side for the 'normal' case.

[OverZealous]

All set, thank you for finding this and for your feedback!

[OverZealous]

Note: you'll have to bump your major version of run-sequence to get this fix, as it was a breaking change.

[poelstra]

Thanks for your very quick fix! I'll be updating tomorrow.

[Delagen]

I am not sure why you don't allow to duplicate tasks. Make it at least optional.
For example:
build
bump
build

the first build is only test (for sure that all compiled)
bump version
the second build to include new version in build

[OverZealous]

@Delagen You can still do this outside of a single sequence by creating a separate task and using normal task dependencies. I don't think that's a strong enough use case vs being more likely that the task was entered in error.

You can also stay on the previous version of run-sequence, which is why this was marked as a breaking change.

[poelstra]

Btw: I tested the fix, and it works like a charm!

I didn't check it, but like @Delagen, I was also expecting the check to only be applied to each 'parallel' member only, so:
runSequence(['a','a'],'b')
would fail, but
runSequence('a','a','b')
would not.

This is in correspondence with the fact that gulp (and runSequence) correctly schedules these tasks,

Example gulpfile:

var gulp = require('gulp');
var runSequence = require('run-sequence');

gulp.task('test', ['x', 'y']);

gulp.task('x', function(callback) {
  runSequence('a', 'b', 'a', callback);
});

gulp.task('y', function(callback) {
  runSequence('a', 'b', 'a', callback);
});

gulp.task('a', function(callback) {
    console.log('a');
    setTimeout(callback, 100);
});

gulp.task('b', function(callback) {
    console.log('b');
    setTimeout(callback, 100);
});

Expected output:

[16:00:43] Using gulpfile c:\Source\testje\gulpfile.js
[16:00:43] Starting 'x'...
[16:00:43] Starting 'a'...
a
[16:00:43] Starting 'y'...
[16:00:43] Finished 'a' after 101 ms
[16:00:43] Starting 'b'...
b
[16:00:43] Finished 'b' after 106 ms
[16:00:43] Starting 'a'...
a
[16:00:43] Finished 'a' after 105 ms
[16:00:43] Finished 'x' after 321 ms
[16:00:43] Finished 'y' after 320 ms
[16:00:43] Starting 'test'...
[16:00:43] Finished 'test' after 17 μs

Note how the a-b-a sequence is in fact run just once, although both x and y independently start such a sequence.

I personally haven't needed this yet, but @Delagen's case sounds reasonable.
I can imagine that things can become messy pretty quickly though, especially when nesting multiple run-sequences etc.
At the moment, I'm neither for nor against either option...

[OverZealous]

Well, it's a relatively easy fix. I guess I don't really care, either way. I can push out a change in a couple of minutes.

[OverZealous]

OK, I pushed a fix out as 1.0.1, which allows duplication of a task as long as it is not within the same parallel set (array).

Sorry @Delagen. I hope this solution works for you.