Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Gulp tasks should be named functions wrapped and without runSequence #289

Closed
dman777 opened this issue Aug 1, 2015 · 7 comments
Closed

Comments

@dman777
Copy link

dman777 commented Aug 1, 2015

The gulp tasks have been written against gulp convention. The true intention of the gulp design is to write regular Javascript functions, and only make the task for calling them. With this format, you would never need runSequence which uses(or used to use) gulp.start().

If you need sync, use a promise library like blue bird performance promise library. I believe Gulp 4.0 has it's own promise's but I prefer performance bluebird.

This is from the authors of Gulp. Sources:
gulpjs/gulp#755
gulpjs/gulp#458
example on proper convention:

function getJsFiles() {
    var sourcePaths = [
        './app/scripts/**/*.js',
        '!./app/scripts/**/*.spec.js',
        '!./app/scripts/app.js'
    ];

    var sources = gulp.src(sourcePaths, { read: false }).pipe(angularFilesort());

    return gulp.src('./app/index.html')
        .pipe(injector(sources, { ignorePath: 'app', addRootSlash: false }))
        .pipe(gulp.dest('./app'));
} 

gulp.task('js', function () {
    jsStream = getJsFiles();
});
@Frikki
Copy link

Frikki commented Aug 9, 2015

👍

@arthurvr
Copy link
Contributor

You're correct, it's kind of a hack. But actually, I think the version we have at the moment is just a little more readable and understandable to most of our users. I haven't followed it all but I think gulpjs/gulp#355 makes this all much easier, so I'd say we should just postpone fixing this issue till Gulp 4 comes out (normally soon). We'll need to do many updates then anyways.

@dman777
Copy link
Author

dman777 commented Aug 18, 2015

Just to add.... it's been a few months but last I checked runSequence used gulp.start() which will be deprecated in gulp 4.0. For this reason, runSequence will break in Gulp 4.0.

@addyosmani
Copy link
Member

You're correct, it's kind of a hack. But actually, I think the version we have at the moment is just a little more readable and understandable to most of our users

👍 This echoes my sentiments too. We could keep this issue open until Gulp 4 is properly out and then tackle it correctly.

samccone added a commit that referenced this issue Sep 24, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes #289
samccone added a commit that referenced this issue Sep 24, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes #289
@samccone
Copy link
Contributor

@dman777 I took an initial pass at this by pulling out non specific logic into functions.
If you would like to take it further / :x: to run-sync which imho is not the worst thing ever PRs are welcome!

samccone added a commit that referenced this issue Sep 24, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes #289
samccone added a commit that referenced this issue Sep 25, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes #289
samccone added a commit that referenced this issue Sep 25, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes #289
samccone added a commit to samccone/polymer-starter-kit that referenced this issue Sep 25, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes Polymer#289
samccone added a commit to samccone/polymer-starter-kit that referenced this issue Sep 25, 2015
Extract non-specific tasks to fns to be more inline with "the gulp way✨"

Fixes Polymer#289
@dman777
Copy link
Author

dman777 commented Sep 26, 2015

@samccone Sorry about the late response. I would but I'm backed up on other projects ☺️ Sorry!

@samccone
Copy link
Contributor

No worries. This is OSS so any help is appreciated

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

No branches or pull requests

5 participants