API: Presets as code, middleware injection

[eliperelman]

In order to support a better middleware story (for reasons below), I think handling requireing should be removed from the Neutrino API, and pushed into the CLI. Basically we switch the constructor to do nothing, and introduce something like .use() for injecting presets. This makes module resolution better for Neutrino, while giving preset authors and extenders a more consistent custom configuration API (i.e. death to .custom).

Prototyping and experimentation is happening at #86.

EDIT: This has evolved into something a little different than proposed below, see comments for current direction. Keeping this here for posterity.

---

Presets as code

The Neutrino constructor will no longer take an array of preset names/paths. Instead, a .use() or similar would accept a preset as code, probably some kind of middleware function:

use(preset) {
  preset(this);
}

package.json configuration

I'm thinking that merging of package.json data should also be moved into the CLI. We already have a mechanism for merging data, so the CLI just needs to call this on the configuration after .useing all the presets

const api = new Neutrino();

api.use(require('preset-alpha'));
api.use(require('preset-beta'));
api.config.merge(require('package.json').config.neutrino);

Along those lines, we should probably change the schema of config data in package.json so it can be aggregated into a single namespace. Looking at html and neutrino and custom and presets et al is...meh.

Maybe just a top-level "neutrino" is OK.

{
  "neutrino": {
    "presets": ["neutrino-preset-react"],
    "config": { "publicPath": "./" },
    "jest": { "bail": true },
    "mocha": { "reporter": "nyan" }
  }
}

Middleware configuration

The .custom configuration object is a hack that needs to go. Making preset authors manually put configuration on this, with no way to automatically merge this back from package.json makes things annoying. This blocks #23, and you can see evidence of this mess in the Jest preset. it would be nice if preset authors could also use the middleware concept to inject preset-specific configuration with automatic merging from additional arbitrary keys from the package.json neutrino object, e.g. jest, mocha. Maybe if .use() takes a key as well.

use(name, preset) {
  const custom = this.middlewareConfig[name];
  preset(this, custom);
}

// ...

module.exports = (neutrino, options) => {
  const mochaConfig = { ...defaults, ...options };
};

---

I know this is a lot of changes, so I'm saving this for v5 (obviously since these are breaking changes). I'd appreciate any thoughts on this, recommendations, or better practices.

[tj]

Sounds reasonable to me! I'd definitely prefer the succinct package.json config vs breaking out into JavaScript, that would start to take away from what makes neutrino attractive. I think just exposing this to the preset is totally fine, let the ecosystem figure out what is appropriate for config properties etc.

Hopefully people will keep that in mind when creating presets and keep config minimal. If the changes are large then they might as well just write a preset.

[eliperelman]

@tj yep, definitely want to keep support for configuring via package.json (exposed at neutrino.config).

The thing that I'm thinking about at the moment is preset config vs. webpack config. Take the mocha preset for example. If the preset sets some defaults for mocha, like reporter: bdd, how can we more elegantly expose that to be overridden from both the package.json and override JS files? Maybe the solution really is just hang some kind of meta object off of neutrino which can be merged.

// neutrino-preset-mocha
module.exports = neutrino => {
  neutrino.options.mocha = {
    reporter: 'bdd'
  };

  neutrino.on('test', args => mocha(args, neutrino.options.mocha)); // pseudocode
};

// package.json
{
  "neutrino": {
    "options": {
      "mocha": { "reporter": "nyan" }
    }
  }
}

// custom-override.js
module.exports = neutrino => {
  neutrino.options.mocha = { reporter: 'nyan' };
};

[eliperelman]

Oops, false close.

[SpencerCDixon]

Maybe just a top-level "neutrino" is OK.

+1 to that

[SpencerCDixon]

If exposing the name override in the middleware concept here:

use(name, preset) {
  const custom = this.middlewareConfig[name];
  preset(this, custom);
}

I would have the preset func arg be first and the name second since maybe not all presets will require and/or allow overrides via package.json.

Hopefully people will keep that in mind when creating presets and keeping config minimal. If the changes are large then they might as well just write a preset.

I think this is an important point. I've actually already thought about breaking up the web preset into smaller presets that can then be composed. For example, when I develop react apps I don't use css and therefore don't really need all the css loaders, etc. Encouraging lots of 'Unix' like presets that just add one bit of functionality will then make it easy for companies/individuals/frameworks to build the perfect 'webpack stack' so to speak.

[eliperelman]

So far I'm finding that name doesn't serve a purpose. I don't have it implemented right now.

[eliperelman]

Spent a good amount of time tonight experimenting with this middleware concept, and so far things look positive. It's entirely possible to break up presets into even more reusable middleware chunks, even ones defined inline. To make it easier to create preset middleware, I changed the signature for a use-able function to:

middleware(config, neutrino)

where config is the neutrino.config webpack-chain instance, and neutrino is the previous top-level API. It's mostly just for convenience. This means that presets are middleware, and that they have the same signature. So in Neutrino:

use(presets = []) {
  if (!Array.isArray(presets)) {
    presets = [presets];
  }

  presets.map(preset => preset(this.config, this));
}

What's useful about most middleware is providing options via closure. Looking at the progress of the v5 branch, where all this experimentation is taking place, it really cleans up the presets (albeit at the expense of many requires, but I digress). In a nutshell:

function factory(options) {
  return function middleware(config, neutrino) {
    // config...
  };
}

Configurable middleware can simply be:

// custom-middleware
module.exports = options => config => config.doSomethingWith(options);

And be used:

const custom = require('custom-middleware');

module.exports = (config, neutrino) => {
  neutrino.use(custom({ alpha: 'beta' }));
};

Non-configurable middleware could easily skip the closure:

const plainMiddleware = config => config.doSomethingPlain();

module.exports = (config, neutrino) => {
  neutrino.use(plainMiddleware);
};

// or use both together

const custom = require('custom-middleware');
const plainMiddleware = config => config.doSomethingPlain();

module.exports = (config, neutrino) => {
  neutrino.use([
    custom({ alpha: 'beta' }),
    plainMiddleware
  ]);
};

---

I think it's interesting that the presets are converging towards looking semi-declarative. Concretely, just looking at the start of the web preset:

module.exports = (config, neutrino) => {
  neutrino.use([
    env(),
    htmlLoader(),
    styleLoader(),
    fontLoader(),
    imageLoader(),
    htmlTemplate(),
    compileLoader({
      include: [SRC, TEST],
      babel: {
        presets: [
          [require.resolve('babel-preset-env'), {
            modules: false,
            useBuiltIns: true,
            include: ['transform-regenerator'],
            targets: {
              browsers: [
                'last 2 Chrome versions',
                'last 2 Firefox versions',
                'last 2 Edge versions',
                'last 2 Opera versions',
                'last 2 Safari versions',
                'last 2 iOS versions'
              ]
            }
          }]
        ]
      }
    })
  ]);
}

Seems a bit easier to reason about.

---

None of this is really revolutionary of course, I just think this would be a big step forward for making presets much more composable. What's great is that none of this really cuts into the existing override APIs; they have an almost identical experience right now with the only change being the first argument being config instead of neutrino.

[tj]

In some ways I think it could almost just be:

neutrino = one(neutrino)
neutrino = two(neutrino)

With options

neutrino = one({ stuff, here })(neutrino)
neutrino = two(neutrino)

With .use() to make things slightly easier to read:

neutrino.use(one({ stuff, here ))
neutrino.use(two)

I typically just did fn(this) like you originally mentioned for middleware. If Neutrino always expects a closure, maybe the config => neutrino => ... signature is all you need, no (config, neutrino), where in the first case config would be what's in the package.json not a separate set of options in JS-land.

Could be fine to avoid use() all together, nice to have the flexibility if you need to do something in there but it does end up looking more magical than just:

one(neutrino, { stuff, here })
two(neutrino)

I think ^ would get my vote unless there's other reasons to go with .use().

[eliperelman]

neutrino = one(neutrino)
neutrino = two(neutrino)

Right now there isn't the expectation that middleware actually returns anything since everything is based on side effects against config or neutrino.

one(neutrino, { stuff, here })
two(neutrino)

I like this as well. I think in this case .use would become straight-up sugar for this.

use(preset, options) {
  preset(this, options);
}

one(neutrino, { stuff, here })
neutrino.use(two)
neutrino.use(three, { stuff, here });

@tj what I like about your suggestion is that it does away with the closure completely, which is a plus for reducing complexity.

[eliperelman]

Using @tj's suggestion, the web preset can take the following form:

module.exports = neutrino => {
  neutrino.use(env);
  neutrino.use(htmlLoader);
  neutrino.use(styleLoader);
  neutrino.use(fontLoader);
  neutrino.use(imageLoader);
  neutrino.use(htmlTemplate);
  neutrino.use(compileLoader, {
    include: [SRC, TEST],
    babel: {
      presets: [
        [require.resolve('babel-preset-env'), {
          modules: false,
          useBuiltIns: true,
          include: ['transform-regenerator'],
          targets: {
            browsers: [
              'last 2 Chrome versions',
              'last 2 Firefox versions',
              'last 2 Edge versions',
              'last 2 Opera versions',
              'last 2 Safari versions',
              'last 2 iOS versions'
            ]
          }
        }]
      ]
    }
  });

  // ...
};

[eliperelman]

A really neat outcome of this middleware approach is that you can really simplify the verbosity of some operations, if you so wish. For example, if I want to easily extend the options of a rule's loader, I could create a generic middleware for doing that:

const merge = require('deepmerge');

const loaderMerge = (ruleId, loaderId) => ({ config }, options) => config.module
  .rule(ruleId)
  .loader(loaderId, props => merge(props, { options }));

So instead of writing stuff like this all over the presets:

neutrino.config.module
  .rule('compile')
  .loader('babel', props => merge(props, {
    options: {
      env: {
        test: {
          plugins: [require.resolve('babel-plugin-transform-es2015-modules-commonjs')]
        }
      }
    }
  }));

You can use the middleware to simplify and do the merging for you:

neutrino.use(loaderMerge('compile', 'babel'), {
  env: {
    test: {
      plugins: [require.resolve('babel-plugin-transform-es2015-modules-commonjs')]
    }
  }
});

😍

[tj]

BTW is the neutrino part even necessary? Maybe just passing config would be nice, then it's not coupled to neutrino, just a function that accepts the webpack chain stuff and you do stuff with it haha

[SpencerCDixon]

Correct me if I'm wrong @eliperelman but I believe the neutrino part is necessary for if the presets want to be able to hook into the build/test/etc. events, yeah?

[eliperelman]

@tj correct, it's not required for just doing configuration. In my earlier comment I had split the signature to middleware(config, neutrino) so you could focus on just config and ignore neutrino if desired. That's still possible.

Having access to neutrino though is still necessary in some situations. For example in the mocha preset, accessing neutrino.options.mocha contains the data from package.json at that spot for passing into the mocha executable. Not to mention, Neutrino is still an EventEmitter for listening when to start testing.

In the eslint middleware, it exposes a method at neutrino.eslintrc() for generating the object useful for consumption by an ESLint RC file.

So TLDR; it's not required if you're only doing configuration, but if you're doing more than that you still need to access the Neutrino instance.

[eliperelman]

@SpencerCDixon yes, that is correct.

[tj]

Ahhh ok cool, I thought config === neutrino.options. Wouldn't the package.json config usually be the same thing you'd pass in JS-land?

[eliperelman]

@tj similar. package.json neutrino.config gets parsed and merged into the webpack-chain instance. So you can either provide it via package.json, or use the chaining API and do the same thing. The JS overrides obviously have more power since you can't remove something from an object by just merging objects.

Also, neutrino.options !== neutrino.config. neutrino.config is the aggregation of all Webpack configuration to be handed over to Webpack. neutrino.options is custom data that can be used by the presets for their own metadata, e.g. neutrino.options.mocha = { reporter: 'nyan' }.

[tj]

Right right, I should probably use it more before I comment haha, just tested it out when I first saw it

[eliperelman]

Resolved with #86.