About customization/extention of commands using "hooks"

[lucasconstantino]

Project setup is a mess because one thing will always interferir with others: eslint my need to use babel's parsing; husky/lint-staged might need to understand there is prettier/eslint available; jest setup needs to know compilation decisions; and so on.

Taking that in consideration, and combining that with the (very nice) idea of presets mrm provides, I would like to suggest we could implement some sort of hook based capabilities for interactions between different tasks - or even global interference, eventually.

One patter I think might suit the current architecture of this project is hooks - or event based alters, for that mater.

There are many parts of the system we could benefit from hooks. For instance, we could have hooks that allow altering formats actions, such as save on JSONs:

module.exports = function(filename, defaultValues, hook) {
  return {
    /** Save file */
    save() {
      const result = hook('format:json:save', json, { filename, defaultValues })
      const content = commentsJson.stringify(result, null, file.getIndent())
      file.save(content)
      return this
    },
  }
}

This would ultimately allow modules/tasks to alter results of JSON file editing (for whatever reason that be, this is just an example):

task.hooks = {
  'format:json:save': (json, { fileName }) => fileName === 'package.json'
    ? { ....json, customField: 'value' }
    : json
}

module.export = task

This, when combining with presets (that will ultimately run multiple tasks together) could mean lot's of decoupling on current code.

This could also allow, for instance, for specific packages (or even user's global setup) to extend specific action on tasks. #48, for instance, could be solved with something similar to this:

function runYarn(deps, options, exec) {
  // ...
  const alteredArgs = hook('npm:yarn:install:args', args, { deps, options, exec })

	return execCommand(exec, 'yarn', alteredArgs, {
		stdio: options.stdio === undefined ? 'inherit' : options.stdio,
		cwd: options.cwd,
	});
}

This could be very simply benefited anywhere, like a mrm-task-yarn-workspace package or the user's global, like this:

task.hooks = {
  'npm:yarn:install:args': args => ['-W', ...args],
}
module.export = task

Disclaimer: I don't think this situation is a good example that should be put in another module. In this specific case, I agree this could be handled by the mrm-core module, as there is definitely a way to determine if we need to set the workspace flag or not. This is only an example of how easy changes like this could become even without the need of PRs or code-change, when many times the involved package might not be the desirable place to some specific customization or when there is no desire to add a configuration to solve such request.

I think if this gets considered, the implementation itself would definitely be retro-compatible, and could happen incrementally for most of the situations. Definitely there could be some code cleaning due to that, and some better separation of concerns in some packages, though that kind of refactor would have to be discussed in each particular case.

[lucasconstantino]

Ps.: I have very diverging feeling on this kind of architecture I'm proposing. I just thought I should share the concept and what the benefits could be, if implemented ;)

[sapegin]

Yeah, this might be a useful feature, though I don't see real use cases so far ;-)

[brettz9]

I love the attention to modularity but I am curious what this might offer beyond what config options can achieve. If authors don't want to be bothered to add config, they might not want to be bothered to add hooks either, I'd think.

For your ESLint/Babel use case, for example, the eslint task could check for babel config to determine whether to add the parser, and the babel task could of course consult its own config. Seems the lint-staged already checks for prettier.

I think a bigger issue is just that the core tasks agree to be as granular as possible (as lint-staged does). For a further improvement of this, I'd suggest readme only add a "Contributing" file, either if there is configFile set, or, if having the config file is desired by default, then allowing a noConfigFile: true option instead.

[brettz9]

Also:

1. The docs should perhaps mention the idea of "namespacing" options, so as to avoid unintentional conflict with other task options.
2. I think a practice the built-in tasks might consider following is to expect explicit config more frequently but to merely log to console if the optional config is missing. Where it is really critical to a task, it could be required, e.g., readmeFile could be required for readme, but when not required, e.g., for the "ci" task optionally adding badges to the README, it would merely log the fact that no readmeFile was found so skipping that. Setting readmeFile to false would avoid such warnings entirely in ci (though not for tasks where it is required like readme). (I guess even the idea of required options could be dropped, such that the readme task could even be made to avoid erring under such conditions, making it easier to merge configs without needing to disable the task if not desired, but that might be confusing as to why a task was running but not having an effect.) But this raises the question too of whether let's say ci should have an option not to add to the README so that if one wanted a README and ci file, but not a CI badge, then one could do so. I think for built-in tests, the more configurable (though with reasonable defaults), the better.

[brettz9]

Here's one use case for hooks where I don't think options would work...

For the readme task, other tasks might wish to advertise that they have some block to insert, such as a badge to reference, and provide the text value for such blocks. The readme task could then populate any template which referenced the registered additional variables.

While the default template would not be able to reference variables it couldn't know about in advance (unless providing a dumping ground where the order of insertion was arbitrary or perhaps task-suggested), this approach does allow for users to fully control the resulting order in the README by defining the template they desire. This may be more important, e.g., if one wishes npm/dependency badges in one place, testing/coverage badges in another place, etc. (e.g., ${npmBadge} ${testBadge} ${coverageBadge} ## Intro)

Users might also take into account in template design that their templates might be reused with or without certain tasks, so there'd be no need to assume a variable's existence.