[Suggestion] Plugin system

[yannisgu]

To achieve a more modular architecture in estatico, I propose to add a simple plugin system to estatico as a first step.
A plugin is basically a Node.js CommonJS which exports a function, which initializes the plugin.
So that estatico is aware of plugins, a configuration file "estatico.config.js" (inspired by webpack) is introduced. The configuration file must export an object in this format:

{
  plugins: {
     pluginName: pluginConfiguration
   }
}

Example:

{
  plugins: {
     "estatico-publish": {publishDest: '/var/www/'},
     "estatico-html-copy": {}
   }
}

When starting, estatico then loads requires every plugin and calls the function with these parameters:

require(pluginName)(pluginConfiguration, gulp)

A plugin then should be built similar to how tasks are built today. The pluginConfiguration coming from the wrapper function should be merged with the taskConfig. Then the gulp-task should be registred with gulp.task() and an object with the metadata for the task should be returned (same as tasks do today).
All these returned objects are then stored in something like a plugin-registry, which then can be used from the watch task, to configure the watch things.

What do you think?

[backflip]

Sounds intriguing! Two thoughts:

• Tasks need to be composable, i.e. you need to be able to define whether you want to run them in parallel or in series
• An external plugin registering a "public" gulp task might be a bit counterintuitive. It could overwrite tasks you register yourself if they have the same name.

That's why I'd rather register and compose them in a gulpfile.js. What do you think?

const gulp = require('gulp')
const html = require('estatico-html')
const validateHtml = require('estatico-html-validate')
const config = require('./estatico.config.js')

gulp.task('html', gulp.series(
  function htmlTask () {
    return html.task(config.html)
  },
  function validateHtmlTask () {
    return validateHtml.task(config.validateHtml)
  }
))

It might even make sense to skip the external config file and have everything in gulpfile.js, but it could be too much.

[backflip]

Example: https://github.com/unic/estatico-nou

Plugins:

• https://github.com/unic/estatico-html
• https://github.com/unic/estatico-html-validate
• https://github.com/unic/estatico-watch

Demo: Writing any invalid HTML should trigger and log an error.

/cc @orioltf, @swey, @marbor3

[yannisgu]

To register and compose all the stuff in the project itself seems reasonable.

How do you plan to implement the watch stuff?
I like the current implementation, which iterates over all the tasks and register those who have a watch-configuration.

[orioltf]

Example: https://github.com/unic/estatico-nou

Did you choose this name on purpose? Lovely catalan suffix! 😍

[backflip]

@yannisgu, I'll make a proposal. I like the current solution, too.

@orioltf, absolutely!

[backflip]

@yannisgu Here you go: https://github.com/unic/estatico-nou/blob/bfc86e8fb5650d2b9939212c0463a75b614588f4/gulpfile.js#L39-L51

[backflip]

What would be the most reasonable approach regarding task options? Does https://github.com/unic/estatico-nou/blob/bfc86e8fb5650d2b9939212c0463a75b614588f4/gulpfile.js#L12-L28 make sense where we merge the custom config with the exposed defaults of our tasks? Or should we rather initialize the imported tasks with the custom config and then expose the result of the merged settings as task.config or something?

Variant 1:

const tasks = { 
  html = require('estatico-html')
}

const config = {
  html: merge({}, tasks.html.defaults, {
    src: '*.hbs'
  })
}

// → Merged html config
console.log(config.html)

Variant 2:

const customConfig = {
  html: merge({}, tasks.html.defaults, {
    src: '*.hbs'
  })
}

const tasks = { 
  html = require('estatico-html')(customConfig.html)
}

// → Merged html config
console.log(tasks.html.config)

The latter one would slightly simplify things like https://github.com/unic/estatico-nou/blob/bfc86e8fb5650d2b9939212c0463a75b614588f4/gulpfile.js#L39-L51 to something like this:

gulp.task('watch', function watchTask () {
  Object.keys(config).forEach((taskName) => {
    if (config[taskName].watch) {
      tasks.watch.fn({
        task: tasks[taskName]
      }, gulp)
    }
  })
})

Independent of the approach, the custom config could live in a separate file instead of being part of gulpfile.js as suggested by @yannisgu.

[yannisgu]

@backflip I would go for Variant 2, so that we have everything which belongs to the html task are in the same object and not something in config and soming in tasks.html
But actually not too important

[backflip]

@yannisgu, i just realized you had already suggested this in your proposal. Sorry. :)

[backflip]

Updated: https://github.com/unic/estatico-nou/blob/79bb6d27232d56a88e40831a1110dfc634b3d630/gulpfile.js

PS: You can run npm update to get the newest task dependencies there.

[lbsonley]

This sounds like a great idea. Is the plan to strip out all tasks into separate modules and deliver estatico without any core functionality?

Personally, I would like to see some tasks remain as the core (for example html, css, clean, serve, livereload, watch, media:copy, js:lint, etc.). Things that don't change depending on the project.

I guess, in order to achieve this, core tasks would be first extracted into their own module, and then these modules would be included in the default package.json file so they are available out of the box?

Or does this contradict the entire exercise of modularizing?

What is the roadmap/game-plan for developing? Should we already start modularizing tasks in separate repos on github.com/unic and test them on estatico-nuo? Then, once we have all tasks in modules, integrate the changes from estatico-nuo into estatico?

[backflip]

@lbsonley, I could definitely make sense to have some of the tasks in there by default (already part of package.json and imported in gulpfile.js).

There is currently no roadmap. We can come up with one if we are sure about the direction we want to go, I guess.

[lbsonley]

Also, if anyone has tried to clone this locally and runs into

/Users/benjamin.sonley/.nvm/versions/node/v6.7.0/lib/node_modules/gulp/bin/gulp.js:129
    gulpInst.start.apply(gulpInst, toRun);
                  ^

TypeError: Cannot read property 'apply' of undefined
    at /Users/benjamin.sonley/.nvm/versions/node/v6.7.0/lib/node_modules/gulp/bin/gulp.js:129:19
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)
    at Module.runMain (module.js:592:11)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3

its probably because your global gulp-cli verson does not support the local gulp 4
i get the following when in the estatico-nuo directory

gulp -v
[11:29:09] CLI version 3.9.1
[11:29:09] Local version 4.0.0-alpha.2

You can either update to gulp-cli 4.0.0 globally
npm install -g "gulpjs/gulp-cli#4.0"
OR use gulp-cli 1.2.2 which supports gulp 4.0
sudo npm install [email protected] -g
OR (my preferred method) access the local gulp version via
$(npm bin)/gulp

source: gulp-cli/issues/9

[backflip]

@lbsonley, you can run npm run gulp instead, it will use the local version.

[lbsonley]

Independent of the approach, the custom config could live in a separate file instead of being part of gulpfile.js as suggested by @yannisgu.

Would this be an acceptable approach to separating the custom configs?
lbsonley/estatico-nou@1e502e9

[backflip]

Absolutely! Though I'd rather use @yannisgu suggestion of a estatico.config.js if we really want to separate it. However, I'm not sure we gain a lot by separating it since you will need to set up gulpfile.js manually anyway. So you'll just have to edit two files instead of one.

[yannisgu]

@backflip is probably right. We don't gain too much by splitting config from gulpfile, when gulpfile needs to be touched anyway.

I made the proposal to have estatico.config.js, because I assumed that every gulpfile.js would be the same for every project. However this is not feasable, as the task setup must be done in the project itself. So yeah, there is no value to split gulpfile and config.
Instead the gulpfile.js should be kept as small as possible.

To answer @lbsonley's question: I would try to extract the gulp-tasks to small, reusable packages, however keep the estatico repository as kind of "starter-kit" you can copy and never update again.
In a second step, also the "preview" part and the framework-part should probably be extracted to their own packages.

[backflip]

The splitting-up has started and will be tracked in unic/estatico-nou#3

[backflip]

@yannisgu, here you go: https://github.com/unic/estatico-nou

I'd very much appreciate any feedback!

[yannisgu]

Very nice, @backflip!
For my taste the Gulpfile.js in the boilerplate is too long, but I didn't had much time to think about better solutions

[backflip]

@yannisgu, I agree that it is rather long. We'll see how we like it on upcoming projects.
Anyway, thanks a bunch for initiating this!