[Idea] Adding / Removing nodes during build

[timmfin]

Recently, I got the time again to dig back into my work on revamping HubSpot’s front end build system. I hadn’t done much with it for a while, and figured I’d start by updating all my dependencies to the latest, the main of which was taking broccoli from v0.14.x to v1.0.0-beta.5.

Overall the changes in v1 seem great, and will hopefully lead to plugins that are easier to write and more performant builds, but it seems like new API made it harder to write utility plugins that compose nodes/trees (as I originally mentioned in this twitter conversation).

tl;dr (because yeah, this is long)

• The problem, pointing out what you could do before.
• What workarounds I've tried to do with no changes to the v1 beta (modifying this._inputNodes)
• Some thoughts on what we could do if we made some small to medium changes to v1, such as: making nodes emit events (example gist) and new _inputNode manipulation methods on broccoli-plugin (example gist)
• Cute turtle gif

/cc @stefanpenner @joliss

---

So what actually is the concern?

Before v1, broccoli did not build the graph ahead of time. And as a side-effect of how that traversal used to work, plugins had the ability to create a Promise on the completion of any particular input node (via the readTree function). So you could compose input nodes at will, like this silly, hypothetical (ES2015-ful) example:

var promiseMapSeries = require('promise-map-series');

function MyPlugin(inputNodes, options) {
  this.options = options;

  // In this example, let's say we need to do something special with the first
  // inputNode as compared to the others.
  [this.firstNode, ...this.otherNodes] = inputNodes;
}

MyPlugin.prototype.read = function(readTree) {
  let outputPaths = [];

  this.options.startCallback();  // first "hook" from plugin options

  return readTree(this.firstNode).then(firstNodeOutput => {
    outputPaths.push(firstNodeOutput);
    this.options.secondCallback();  // another "hook" from plugin options

    return promiseMapSeries(this.otherNodes, (n) => {
      return readTree(n).then(out => {
        this.options.postEachOtherCallback();  // yet another "hook" from plugin options
        return out;
      })
    });
  }).then(([...otherNodesOutput]) => {
    outputPaths = outputPaths.concat(otherNodesOutput);
    this.options.finalCallback();  // a finished "hook" from plugin options
    return whateverWeWantToDoWith(outputPaths);
  })
}

That example is way over the top, but it shows how a plugin could compose a various number of input nodes and call any code/hooks (whether it was debugging statements or actual plugin manipulation logic) as it desired. 

However, in v1 I see no straightforward way to write a plugin like this. You don’t have access to your inputNode’s promises or any of the builder’s beginNode or endNode hooks. Yes it is totally true that the readTree-style code is darn hard to follow, and the v1 API makes most plugin implementations much clearer; nonetheless, I’d argue losing this compositional flexibility is a bummer. I don't think missing it is a "show stopper", but I do think broccoli would be less powerful and miss out on a whole set interesting/useful (edge-case?) plugins without it.

---

What can we do with v1 beta as is

Alrighty, time to be constructive. Let’s experiment and see what we can do with v1 before trying to change it. The only two real avenues I saw as possibilities were:

1. Adding or manipulating a plugin’s inputNodes before the builder creates its topologically sorted list.
2. Wrapping and replacing the build method on other inputNodes to hook into their execution.

Originally, I started to mess with the latter, but it wasn’t working very well. It was too easy to goof things up, like not apply the original build method with the correct context. And from a debugging perspective, having other code replace your function was a pain in the butt.

So I gave manipulating inputNodes, more specifically manipulating the “private” passedInputNode._inputNodes array created via broccoli-plugin, a shot. You can see a realistic example of this in before-build.js on the experimental move-to-broccoli-v1.0.0 branch of my broccoli-stew fork. Here is the core of what is going on with extraneous stuff removed and plus some comments:

function beforeBuild(node, cb, options) {
  // Get the last "child node" from our passed node's array of "children" inputNodes
  // (see https://github.com/broccolijs/broccoli-plugin/blob/master/index.js for
  // more context)
  var lastChildNode = node._inputNodes.pop();

  // Take that child node and wrap it with our own, new plugin. I'm using 
  // timmfin/broccoli-quick-plugin as a shortcut so we don't have to create a
  // new constructor with all the boilerplate to extend broccoli-plugin inline.
  var wrappedChildNode = QuickPlugin([lastChildNode], {
    build: function() {
      cb();
    },
    name: "beforeBuild",
    annotation: (options || {}).annotation
  });

  // Next we add our wrapped child node back to the end of the "children" list
  node._inputNodes.push(wrappedChildNode);

  // Lastly return the modified node. When the builder builds its sorted array
  // of the whole build graph, we will now have a "hook" that is run before the 
  // build method of the original node passed to this function.
  return node;
};

This still is pretty hacky, since we are messing with someone else’s “private” _inputNodes array. Though it is less hacky than overwriting the build method on other plugins. So if we had no other option, this seems to be best route to doing this sort of thing in the v1 beta.

---

Hypothesizing changes to v1 beta

Next, let's think about what broccoli core changes we could make to enable easier plugin composition/hooks. Here are some "straightforward" considerations:

1. Make broccoli-plugin mixin RSVP.EventTarget (or similar event system). And while iterating over nodeWrappers during the build, do a nw.originalNode.trigger('begin/endNode') if the trigger method exists.
2. Similar to the above, but instead of mixing in RSVP.EventTarget to broccoli-plugin, add a few more things to the object returned via __broccoliGetInfo__. For example, beginBuild and endBuild keys that can be a list of functions (that the builder will call later).
3. Make the builder assign each originalNode a currentBuildPromise method. It would return a promise that is resolved when its build is finished. And the start of every build would need to clear those promises out and create new ones.

However, for any of those to fully work, we'd also need a hook for each plugin to know that a another rebuild has started (or an existing build finished). Because we may need to re-create event handlers or promise chains for every new build.

Here is a rough thought experiment of a plugin using #1 (likely similar to 2 and 3 above anyway). But it still feels a bit off to me. Forcing the plugin to have to worry about reseting state/handlers every rebuild feels problematic.

Yet another approach would be to go back to input node manipulation, but officially condone it with new methods on broccoli-plugin. For example:

• Plugin.prototype.appendChildNode that will append a new node to the end of this._inputNodes for you (possibly automatically using something like broccoli-quick-plugin if you only give it a function instead of a actual node instance?)
• Plugin.prototype.wrapChildNode that will replace node at index X of this._inputNodes with a new wrapper
• Other similar functions, like insertChildNodeBefore(childNode)
• A Plugin.prototype.wrap that will return a new node that wraps this for you (or maybe better to keep that separate function/plugin, wrap(plugin, ...) kinda like broccoli-quick-plugin)?
• Plus to make wrapping nodes more palatable, add a new nodeType to the builder. Right now there are source and transform types...
  • We could add a hook or function node type whose sole purpose to execute a bit of code.
  • That means those nodes don't have to worry about inputNodes or the filesystem at all, making them easier to add without disrupting the existing build graph.

Here's a thought experiment using those methods. That code seems less complicated, since you don't need to worry about resetting things every build, but it still has some complexity—namely understanding how the order/composition of _inputNodes affects the order of execution during the whole build.

And at this point, I'm getting really handwavy, so I'll stop my ramblings.

[joliss]

To answer this properly, I should probably explain/document the new __broccoliGetInfo__ API. That API would be the correct way to interact with nodes and wrap them. There's some other stuff I want to do first, but think I can get to it within a week.

[timmfin]

Sounds good to me, thanks!

[stefanpenner]

@timmfin Although this wont be an immediate thing for me to work on, I believe exploring this line of thought will be good. There are many cases, especially while discovering new files or projects to import based on contents of existing files, where building the tree dynamically would be beneficial.

[timmfin]

@stefanpenner ok, sounds good.

Though I haven't been using Broccoli for over a year now, so please feel free to close, move to a different issue, or whatever you think is best (but I appreciate the revisit nonetheless!)