implement .observe()

[Rich-Harris]

Thought I'd have a quick crack at something talked about previously on #31, #34 and #28 (comment) - giving each node an .observe() method that gets called when its input successfully builds, in order to do things like linting, style checking, tests, writing intermediate stages to disk for inspection, etc etc etc.

It doesn't make any progress towards the design outlined by @evs-chris in #28 (comment) - for example transformations aren't expressed in terms of observers, so there's a bit of duplication that would be unnecessary with a better architecture. My thinking is that we can hone the API through use first and improve the implementation later.

Example using gobble-eslint:

es5 = gobble( 'src' )
    .transform( 'babel', {
        whitelist: babelTransformWhitelist
    })
    .observe( 'eslint', {
        reportOnly: true // won't fail the build
    });

This lints code after it's been transformed by babel. That's useful because it means you can prevent things like IIFEs in for loops, which can easily happen if you use block binding, even if the original code is perfectly valid. It outputs something like this:

Couple of points:

• It's likely that some tasks would only be relevant in certain situations. At the moment, if the options object has __condition: false, then node.observe(fn, options) === node - i.e. it gets skipped. Example on eslint. Does that make sense, or is it a bit weird?
• The ideal scenario would be that only changed files get linted (or whatever). At the moment that's tricky, because gobble's change detection is somewhat naive - if a sourcemap-generating file transformation gets re-run, all the files are deemed to have changed because the sourceMappingURL location gets bumped, even if the contents haven't changed.
• On a related note, one of the proposed uses for this API is rerunning unit tests only when needed. Even if the change detection were more robust, this is tricky because we need to know which modules each test file depends on, and determine whether they have changed. This likely isn't a problem to solve.

Any feedback welcome!

[evs-chris]

I like it!

The __condition on observers does feel a bit weird, but it doesn't seem like an reasonable stop-gap on the way to a better API. For the eslint example it seems to me that it would be better to have the user make that explicit. For instance, in my builds I usually have and if (gobble.env() === 'production') section that handles minification and whatnot, and that section usually has a complement for dev mode stuff. Having the build skip steps automatically based on how it's called without any other instruction from me directly seems a bit surprising. I do see the value of having it 'just do the right thing', where the 'right thing' is right in 95% of all cases too.

For rerunning select tests, I'd say leave that to the user too. There are too many different ways to set up source and tests to guess which tests need to be rerun when certain sources change. It might be nice to have a specialized observer for those that did a one-to-one module source folder to test folder, though.

+ src
|--+ foo
|--+ bar
|--+ baz
+ test
|--+ foo
|--+ bar
|--+ baz

gobble('src').observe('testModules', { tests: gobble('test') });

As you say though, that's not really a problem for the API to solve.

One thing I've run into with the server scenario is that the downstream transforms aren't notified when there's a failure, so it leaves the server running on a subsequent failed build. That's not necessarily a bad thing, but it can be a bit confusing if you don't keep an eye on your build output. It might be nice to allow downstream observers to get a failure notification so they can clean up and not leave stale or incorrect stuff sitting around if they need to do so.

[Rich-Harris]

After sitting with it for a couple of days I reached the same conclusion - the __condition thing is weird as hell. Not sure what I was thinking. I reckon this could work though:

module.exports = gobble( 'src' )
  .transform( someTransformer, {...})
  .observeIf( someCondition, observer, {..})
  .transform( someOtherTransformer, {...});
  // ...

// equivalent to
var node = gobble( 'src' )
  .transform( someTransformer, {...});

if ( someCondition ) {
  node = node.observe( observer, {...});
}

module.exports = node
  .transform( someOtherObserver, {...});

This isn't such a contrived example - I often find myself storing temporary references so I can do things like...

if ( gobble.env() === 'production' ) {
  node = node.transform( 'uglifyjs' );
}

...which is basically the same thing. And a .transformIf(...) method would be a nice counterpart to .observeIf(...). Does that sound like a better solution?

It might be nice to allow downstream observers to get a failure notification

Yeah, I agree.

[evs-chris]

👍