Bring in new task system (bach) #355

Closed
contra opened this Issue Mar 20, 2014 · 63 comments

Projects

None yet
@contra
Member
contra commented Mar 20, 2014
  • Confirm 100% code coverage/test coverage on bach + child modules
  • Switch task system to bach (+ registry layer)
  • Update tests to use new task system
  • Update documentation folder

Does anyone care about being backwards compatible for a little bit or should we just rip this off like a bandaid?

@contra contra added this to the gulp 4 milestone Mar 20, 2014
@phated phated was assigned by contra Mar 20, 2014
@phated
Member
phated commented Mar 20, 2014

Rip off the bandaid

@contra
Member
contra commented Mar 20, 2014

πŸ‘

@irrg
irrg commented Mar 20, 2014

Rip it off, for the sake of progress!

@tkellen
Member
tkellen commented Mar 20, 2014

Rip off the bandaid.

Bach looks great. Nice work @phated!

@OverZealous

+1 for ripping off the band-aid.

Would it make sense to include a once() method in Bach as was being discussed on issue #347? Basically, a wrapper around an async method that guarantees it will only be run once.

@phated
Member
phated commented Mar 20, 2014

@tkellen thanks, much appreciated

@sindresorhus
Contributor

Rip it off.

@terinjokes
Contributor

πŸ™€

@jarvys
jarvys commented Mar 23, 2014

Where can we see any progress?@Robich's develop branch?

@tomasdev

πŸ‘ ye-yeah

Yeah

@phated
Member
phated commented Mar 26, 2014

probably better if you look at the blaine branch

@jasonrhodes

@phated Looks like you're adding bach as a dependency of orchestrator right nowβ€”is the idea to make a gulp > orchestrator > bach dependency chain or gulp > bach?

@phated
Member
phated commented Mar 27, 2014

@jasonrhodes gulp > orchestrator > bach + registry

@gaboesquivel

+1 for .once() and...
image

@aeharding

+1 for series/parallel/once.

I'm so excited! I have a huge gulpfile already (trying to move from Grunt), and this will make things easier and more reliable without having to spend a week getting comfy with orchestrator. The old way of doing series with dependencies wasn't good enough... It added a lot of repetitive code (spaghetti too to maintain/debug), increasing exponentially with each task you add...
yay

Rip it off! πŸ₯

@sindresorhus
Contributor

What's the status on the new task system?

@phated
Member
phated commented May 16, 2014

@sindresorhus I wrote some tests last weekend, feel free to add more.

@contra
Member
contra commented May 22, 2014

@phated Can you put some of the info from the previous discussion here so when I point people to this issue they understand what the new task system is all about?

@contra contra referenced this issue May 29, 2014
Closed

desc() method #491

@holic
holic commented May 30, 2014

πŸ‘

@phated
Member
phated commented Jun 4, 2014

My proposed solution is to remove that array of deps, but the same effect can be achieved as such:

gulp.task('task1', gulp.parallel('some', 'deps', 'here', function(cb){
  cb();
});

As you can see, this removes the run APIs all together, as we are just composing functions into a single callable function.

var aParallelFunction = gulp.parallel('some', 'task', 'strings', 'here');
aParallelFunction(function(err, res){
    // err will be the errors that occurred
    // res will be the results of called functions
});

An API like this would allow for things like:

// an alias
gulp.task('default', gulp.series('clean', gulp.parallel('js', 'css', 'img'), 'watch'));

Which would run clean first, then js, css, and img, in parallel, then watch last.

A watch task could look like:

gulp.task('watch', function(){
    return gulp.watch(['**'], gulp.series('default', 'notify'));
});

The main concept behind these APIs are composability and that they are just callable functions. You might think it is adding extra verbosity, but I feel like it is less than the current beta APIs are adding.

An added bonus to this approach is that the task management layer doesn't need to account for AOP concepts.

var gulp = require('gulp');
var meld = require('meld');

function before(){
    // do stuff before
}

gulp.task('default', meld.before(function(cb){
    // our task definition
    cb();
}, before);
@anru
anru commented Jun 9, 2014

+1 for this PR!

Simple declarative tasks dependency does not enough - nested gulp.start (or gulp.run) need to do beautiful composition of tasks, especially in case when run or not one task (with their dependencies) depends from results of another task

@contra contra referenced this issue in robrich/orchestrator Jun 10, 2014
Closed

`.start()` should create a nested orchestration #15

@thejohnfreeman

Is all this stuff available from a branch somewhere? I don't see a blaine branch anywhere. Is git://github.com/robrich/gulp#develop still in a good state?

@contra
Member
contra commented Jun 11, 2014

@thejohnfreeman No, that branch robrich has set up is not going to be used. We are going a different direction with the task system, I'm not sure where @phated has his code up

@binarykitchen

Oh series/parallel is exactly what I need right now. Or even better, look over at npm's async` module. They already have dealt with some of these issues.

@jtomaszewski

@binarykitchen , currently you can achieve series/parallel effect with run-sequence plugin .

@binarykitchen

Wicked, thanks @jtomaszewski

@marcelinhov2

@jtomaszewski how can I run in sequence, tasks that run sequence with the run-sequence plugin?

@RnbWd
Contributor
RnbWd commented Jul 17, 2014

I've recently stumbled upon a minimalist task manager called bud. I was able to transfer my gulpfiles into bud within a few hours, and I've actually been using it as a task manager for the last 2 weeks. I replaced gulp with vinyl-fs, and the entire gulp-ecosystem seems to work fine. Two differences to note:

  • Returning a callback is the only way to chain tasks together
  • There's built-in support for using child-processes (exec)

Regarding usage, it feels almost identical to using vinyl-fs/orchestrator. Using 3rd party api's inside tasks (rimraf, browserify) works the same, but I've found that using exec makes my code easier to read and manage. Overall I don't think it should replace orchestrator for gulp, but maybe parts of it could be used as inspiration to help design a better task system.

@phated
Member
phated commented Jul 17, 2014

@RnbWd thanks for the link, but the proposed changes for gulp4 already put more emphasis on standard node than bud does. We are going to support returning an exec or spawn from the callback, so no need to pass that into a task.

@xixixao
Contributor
xixixao commented Jul 18, 2014

@contra Where can I see your progress? You are pointing to this issue from many other places across the ecosystem but it is unclear what the resolution is.

@contra
Member
contra commented Jul 19, 2014

@xixixao I'm not working on this, @phated is

@pkozlowski-opensource
Contributor

@phated do you have more details regarding your proposal available anywhere? I love the general idea / direction but would like to know if you were planing to support functions as arguments to gulp.series / gulp.parallel instead of named tasks only, so one could write:

function doSomething(done) {
    //do something async
    done();
}

gulp.task('default', gulp.series('clean', doSomething, 'watch'));

This would enable "private" tasks (doSomething function) which I would love to have. Obviously the doSomething function could mark the end of the async processing either by invoking a callback, returning a promise, a stream etc.

I'm currently playing with a minimal implementation of this idea so would love to understand better where are you guys heading. Also got some free cycles so willing to help with the spec / impl etc.

@contra
Member
contra commented Jul 19, 2014

@pkozlowski-opensource yes, anonymous functions will essentially be a "private task" - named functions will be callable from the CLI and by identity

@pkozlowski-opensource
Contributor

@contra by saying "named functions" are you referring to the ones "defined" with gulp.task like today:

gulp.task('doSomething', function(done) {
    //do something async
    done();
});

Or are you pondering dropping gulp.task altogether and going with something like in this proposal? #347 (comment)

@contra
Member
contra commented Jul 19, 2014

@pkozlowski-opensource By "named function" i mean the JS definition

function fnName(){
  // code
}

which creates a function with a .name attribute

Any function with a .name attribute, or anonymous functions defined with a name (gulp.task('yo', fn)) will be public and addressable by name

@pkozlowski-opensource
Contributor

@contra OK, thnx for clarifying, much appreciated. The picture starts to get clearer now to the point that I'm eager to play with sample implementations, if there are any. @phated do you have any draft code around?

Now, I'm not sure how I feel about exposing JS function definitions to the CLI - I kind of like how gulp.task makes it explicit what is callable from CLI. Exposing any named functions feels non-obvious to me at this point. But it might be just that I'm used to the current system so feel free to ignore.

@phated
Member
phated commented Jul 19, 2014

@pkozlowski-opensource gulp.task will still be used to register tasks within the task system. gulp.series and gulp.parallel will just be normalizing strings into tasks defined by gulp.task. Any functions (anonymous or named) will still be part of the execution but not registered with the task system, ala private tasks (just functions).

Work is being done in a variety of places. The most stable being https://github.com/phated/async-done which will control the task completion aspect (promise, stream, callback, etc). Something that is pretty far along is https://github.com/phated/bach, although the docs are out of date. Currently, bach relies on async-done, https://github.com/phated/async-settle and https://github.com/phated/async-time, but I am not happy with async-time, or the fact that bach is sometimes-an-eventemitter, sometime-not. The work of gluing those pieces together is happening in https://github.com/orchestrator/orchestrator/tree/blaine but I don't think it has been updated to work with the newest bach changes.

@pkozlowski-opensource
Contributor

@phated awesome, thnx for all the pointers, I've got now nice stream of code to digest. Thnx guys for all your pointers, this is much appreciated - I know that it is not easy to share half-baked stuff.

@dallonf
dallonf commented Jul 23, 2014

What's the ETA on this? I just tried out Gulp today and already my gulpfile is getting cluttered with //temporary workaround until Gulp 4.0 comments. Would love to get those removed!

@phated
Member
phated commented Jul 23, 2014

@dallonf ETA is based on community involvement and contributions or my schedule magically opening up (hint: not going to happen).

@pkozlowski-opensource pkozlowski-opensource referenced this issue in gulpjs/bach Jul 26, 2014
Closed

Update docs #2

@pkozlowski-opensource
Contributor

@phated regarding "community involvement" - I would be very much interested in helping out and invest (significant) amount of time into it as I live very much what I see in Gulp3 and with all the changes coming to Gulp4 it can only get better. To speed up things it would be great if you could try to list / open issues in the corresponding repos for al the "unresolved problems". I fully realise that some of them might be "philosophical" / design problems that are not easy to capture, but having them noted down somewhere would help.

For now my understanding of the various elements is the following one (would be grateful for corrections):

  • https://github.com/phated/async-done - concerned with wrapping asynchronous functions in order to execute them and capture their completion / error. The main idea is to abstract "asynchronicity" so functions can mark their success error via: a callback, promise, stream, rkjs, process etc. This part was extracted from the current orchestrator.
  • https://github.com/phated/bach - function orchestrator - makes it possible to combine async functions (as described above) so those are executed in a sequence / parallel
  • https://github.com/orchestrator/orchestrator/tree/blaine - task orchestrator - builds on top of function orchestration and introduces notation of a task and tasks repository. Pretty much core of the today's orchestrator business.

If the above picture in my head is correct (sorry if I got it all wrong...) I'm still struggling with finding the right place for async-time and async-settle.

async-time

If I understand correctly the idea behind this one is to time asyn functions execution. If so I'm not sure if you are planning to plug this timing facility on the task level or at any async function level. In other words: are you planning to make it possible to time "anonymous tasks" (async functions that are not registered with orchestrator.add)?

async-settle

OK, so I'm probably going to sound stupid here but I'm not sure what is the primary, functional use-case for settled sets of tasks. I'm sure there are some...

I guess I would have more detailed questions regarding the design / implementation. But for now it would be great to have some comments on the above + some info regarding remaining tasks / unresolved design decisions. Maybe it would make sense to hang out on irc / chat / whatever at some point to discuss with people who might want to help out?

@phated
Member
phated commented Jul 26, 2014

@pkozlowski-opensource you are right on all fronts. async-time is my current implementation of timing, after having looked into doing it everywhere else. If a timing wrapper was added in .task (register a task) then we wouldn't time plain functions plugged into a series/parallel call. If the timing wrapper was added in series/parallel call, all orchestrations would have to go through them (this option is something I still don't hate, not sure why I abandoned it). Settled tasks allow for partial builds, which I explained more in gulpjs/bach#2

@pkozlowski-opensource
Contributor

@phated ok, cool, happy to hear that I'm not (totally) lost here. I don't think that I've got solution for the timing question, but in order to see how this all could work I've started putting together a simple POC https://github.com/pkozlowski-opensource/gulp4-poc/blob/master/gulp.js. While doing so bumped into robrich/orchestrator#51 but I guess it might be me doing sth wrong...

@pkozlowski-opensource
Contributor

So, after playing with all the elements of the new gulp task system I must say that @phated did grand job here! For anyone interested in looking into what Gulp4 file might look like, here is a (working!) example:
https://github.com/pkozlowski-opensource/gulp4-poc/blob/master/gulpfile.js

From what I can see / understand there is one problem to solve: timing tasks. On top of this there are number of cleanups / corner-cases handling items to tackle but yeh, the main idea seems to be here and I must say I totally love it. Hope that introducing timing tasks won't turn things upside down!

I'm planning to switch to this POC in the real projects in the coming days / weeks and tackle all the issues I can find but again, the main building blocks seem to be in place :-)

@contra
Member
contra commented Aug 12, 2014

@pkozlowski-opensource Yeah, @phated did do an awesome job with this concept. I have to say I'm enamored with bach, I just hope we can get it released ASAP before people get too cozy with the current system. Timing tasks is difficult, I think @phated had some ideas about that - the last time we spoke about it I think that logic was going to end up in the registry

@mlmorg
mlmorg commented Aug 28, 2014

Would it be possible to peel apart task series/parallel functions in order to add/remove additional tasks to them with this new approach? In other words, can you alter a task function once it's been created?

For example, let's say an external library sets up a build task that has a default set of dependent tasks it runs in series (for example, clean, compile and deploy). Would it be possible to step into that task function and alter it to run a versioning task between the compile and deploy steps?

@phated
Member
phated commented Sep 4, 2014

@mlmorg no, but composition based on environment, etc, should be easy with if/else, arrays and .apply.

@acdlite
acdlite commented Oct 5, 2014

I'm late to this party, so forgive my naivety, as suspect what I'm about to suggest has already been considered and rejected... but why not just use promises? If we bring back gulp.run and have it return a promise, it seems to solve this issue entirely. Plus, there already popular many libraries that deal with async coordination β€” and with promises specifically, like bluebird and co β€” that it feels unnecessary for gulp to reinvent the wheel with its own API. I've been using promises to manage gulp tasks for a while and it works like a charm.

Am I missing a use case that promises don't solve?

@heikki
Member
heikki commented Oct 5, 2014

I have no opinion related to promises.. just pointing to one place I remember same thing being mentioned :). On the other hand I'm happy with the current plan because it matches node-async syntax (series/parallel).

@coen-hyde

I'm super excited about this new task system! Is the Gulp 4.0 branch ready for beta users?

@pikeas
pikeas commented Oct 11, 2014

+1. I've followed the trail of breadcrumbs around Github, but I can't figure out where Bach and/or Gulp 4.0 are being worked on!

@phated
Member
phated commented Oct 11, 2014

There is a branch called 4.0. I'd probably start there?

@alvint
alvint commented Dec 2, 2014

While working on this, it would be nice if forward references to tasks were allowed. See #802.

P.S. - Great job so far!

@demoneaux

Any ETA for Gulp 4 to be released?

@contra
Member
contra commented Dec 21, 2014

@d10 Trying to get it finished out before the end of 2014 but the holidays will probably have everyone preoccupied. I'm in the middle of moving our company across the country so I haven't had a ton of time to put towards closing out 4.0 issues

@demoneaux

@contra thanks for the update! You should probably tag this issue with gulp4.

@therebelrobot

bump :) totally stoked for gulp 4

@contra
Member
contra commented Jan 27, 2015

If anybody wants to help ship this faster: https://github.com/phated/undertaker/issues

@phated
Member
phated commented Mar 22, 2015

Closing this because the 2 remaining items are separate issues now: #978 and #861

@phated phated closed this Mar 22, 2015
@ilanbiala

Can you update the original comment to get rid of the remaining items so that this issue seems complete?

@phated
Member
phated commented Mar 22, 2015

@ilanbiala done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment