RFC: Task / Plugin Unification

[timhall]

RFC: Task / Plugin Unification

[lukeed]

Wow! Thank you for writing this up! This is the first RFC I've received, and it's pretty awesome 😄

Here are my initial thoughts, I may be wrong or misspeak as I go 😇 But I'll definitely be revisiting this a few times as it sinks in & I can think about it more:

1. Unifying tasks and plugins should encourage task reuse (as plugins)

   This is already easily done, I just haven't been good at pointing it out in the docs and example kits.

   // taskfile.js
   exports.styles = require('@lukeed/sass-task')
   exports.scripts = require('@lukeed/babel-task')
   
   // sass-task
   module.exports = function * (task, opts) {
     yield task.source(opts.src || 'src/**/*.{sass,scss}').sass({ ... }).target('dist/css');
   }

   Some work could be done on supplying source and target values (eg, globals?), but that applies to your suggestion too.

2. Your proposed method of chaining tasks (task.run(a).run(task => b(task))) would infinitely nest tasks. For example, the next layers down:

   task.run(a).run(task => {
     return b(task).run(task => {
       return c(task).run(task => {
         return d(task);
       });
     });
   });

   That's assuming task.run(a) even works as is -- it'd likely need to be a(task) like the others.

   That said, your scenario could work if run were a recursive loop, but the main issue is how to set up the loop such that run knows when a new chain-segment is waiting to be run. That's a massive, breaking rewrite on its own.

3. Thanks for the gist 🙏

4. That every utility would need to accept a globs vs files modifier.

5. Pairing 3 & 4 makes me concerned for memory utilization. These two points, alone, add a lot more returned functions to the mix.

6. By returning the resulting task from source -> babel, the parent can continue chain

   This can already be done. All segments of a chain are Promises, as I'm sure you know. A chain can continue after any returned segment, even after a target().

   const js = task => task.source('js/**/*').babel();
   
   exports.build = function * (task) {
     yield js(task).target('build');
   }

   This also works:

   export async function js(task) {
     return task.source('js/**/*.js').babel();
   }
   
   export async function build(task) {
     await task.start('js').target('dist/js');
   }

   My first example hoisted it to a helper function (instead of an exported task function) because IMO there's no point in exporting the js task since it achieves nothing.

7. I appreciate the consideration towards #289, but that's not the issue in this case. I've had it on my list for a while, just haven't had time.

   New Tasks are returned. The issue is how the Tasks are booted. The boot method writes into the same Promise.prototype, which then gets shared across tasks. This, then, is what "muddies" (as I like to call it) the source info (_) across tasks.

   I'm fairly sure I just need to alter the boot method to ignore certain keys (eg _) and/or handle functions only.

8. I'm not sure I understand the point of merge --- at least, not how it's presented.

   Your illustration is just a parallel chain. Nothing is "merging" as I can see.

   However, this makes me think of an optimization for my first bulletpoint:

   // taskfile.js
   const sass = require('@lukeed/sass-task')
   const babel = require('@lukeed/babel-task')
   
   exports.styles = sass
   exports.scripts = babel
   exports.build = [sass, babel] //<= "merge"

   This can be achieved (as presented) just by adding a condition for Array-type tasks. I assume this would be a parallel sequence.

9. The conditional chains are also easily achievable right now. Even with your syntax, it could be a simple plugin. In fact, I had a taskr-if plugin on my low-priority todo list for a while.

   taskr.if(condition, isTruthy, isFalsey)

   As it is, I've taken the lazier route & just run a simple if-block:

   if (true) {
     yield task.start('foobar');
   }

   Its "cool factor" is lacking, but that's about it -- works well 😆

I played with a similar approach before arriving at Fly 2.0. My "big idea" was to make the Task class completely stateless, and instead be a factory function that runs a function with given values. The problem was that run (and just about everything else) had to retain some knowledge about its values... hence state.

The only form that "worked" massively spiked in memory usage, and Taskr itself added a couple of pounds.

This "final form" has been in production for a year now & has been performing exceptionally well across multiple use cases.

I'm not saying "it will never change!" by any means -- however, I fundamentally disagree with (what I think is) the premise of this RFC:

Plugins and Tasks are not the same thing, by design. Tasks encapsulate plugins, which then allows you to run anything within a Task under any mode or condition. And because of this, Tasks themselves are quickly portable and interchangeable.

Plugins, by design, "globally" add or alter what Tasks can perform. Tasks perform and can tell others how to perform, too.

Gunna interrupt myself here; but the main the point is that Tasks and Plugins are two very distinct vocabularies and therefore deserve slightly distinct definitions. This line in the sand makes & keeps it clear in discerning what-is-what... but it's also only a line in the sand, as opposed to a solid brick wall. There's still plenty of room for flexibility, extensibility, and modularity when it comes to Taskr imo.

As we've both pointed out, everything presented (except merge) can be done today. Any extra convenience layers can be wrapped up in a plugin. And any tasks can be wrapped up & shipped around too.

Please let me know if I missed something important, or just flat-out misunderstood a point. 😆

Thank you again 🙌

[timhall]

Thanks for such a detailed response! It's always nice to get a picture into a project's philosophy, I can definitely see the wisdom in explicitly separating the concerns of tasks and plugins. I'll clean up my thoughts based on your response and dig a little deeper into the source since it seems like a good amount was existing (it may just be a matter of clearing it up in the docs).

[timhall]

After thinking about it more based on your responses, what I was thinking can be boiled down to one suggested change:

Allow function of string option for start, serial, and parallel

This would go a long way to composing tasks without having to export everything including tasks that are not meant to be called directly.

function * build(task) {
  yield task.parallel([js, css]);
}

// "internal" functions 
function * js(task) {}
function * css(task) {}

module.exports = {
  build
};

On the question of merge, it was intended to be used in plugins like rev that need access to the output of many separate tasks (e.g. html, js, and css). The results from multiple tasks are merged into a single files/globs and processing continues. Based on your responses, this should be doable with the current system:

module.exports = {
  name: 'merge',
  every: false,
  *func(files, tasks) {
    // key is `this` = `task` in plugin, so chaining is available with `this.start`
    // (might be able to do this with parallel, not sure what is returned though)
    const results = yield Promise.all(tasks, task => this.start(task));
    for (const result of results) {
      // Merge result._.files/globs into this._.files/globs...
    }
  }
};

Great work on the library, really nice to find out most of what I was looking for was already available!

[lukeed]

Hey, great! That looks like something that can be done 🎉

Do you mind copy/pasting that "pass a function" bit into a new issue? We can close this & it'll be easier for tracking.

Yes, that way of "merging" can be done right now. However, the Promise.all you wrote needs a bit o' fixing:

Promise.all(tasks.map(task => this.start(task));
// or
Promise.all(tasks.map(this.start))

[timhall]

👍 #292