indexzero/broadway

Add ability to define plugin `npm` dependencies

Marak opened this issue · 7 comments

I've been running into a common idiosyncrasy with plugins that require dependencies. Looks like a chance to improve the API / set a best practice.

Take for instance, a jade plugin:

/**
 * Jade broadway plugin.
 */

var jade;

exports.attach = function (options) {
  this.jade = {
    render: function (view, data, cb) {
      var html;
      try {
        html = jade.compile(view.template, options)(data);
      } catch (err) {
        if (cb) { return cb(err); }
        throw err;
      }
      if (cb) { return cb(null, html); }
      return html;
    }
  }
};

// `exports.init` gets called by broadway on `app.init`.
exports.init = function (done) {
  try {
    jade = require('jade');
    done();
  } catch (err) {
    done(err);
  }
};

Notice App.init contains a try / catch block around the require statement to determine if jade is available.

It seems this is going to be a common problem ( plugins that require dependencies ).

A good potential solution would be being able to specify npm dependencies on a per plugin basis.

// `exports.dependences` contains hash of npm deps for plugin
exports.dependences = {
  jade: "*"
}

Then, broadway and flatiron would intelligently be able to detect / load modules based on this new information.

The dust template engine seems to present an interesting case as well...

// `exports.init` gets called by broadway on `app.init`.
exports.init = function (done) {
  try {
    dust = require('dust');
    done();
  } catch (err) {
    try {
      dust = require('dustjs-linkedin');
      done();
    } catch (err) {
      done(err);
    }
  }
};

Might there be a way to handle a sequence of fallbacks in case one or more dependencies fail?

@eschmitt - Exactly. We don't want to be reimplementing try / catch / require / async logic in every plugin.

Unless there is another solution for doing this that I'm not aware of, we are going to move forward on the convention of exports.dependencies ( which follows the npm standard ).

@Marak I'm not sure whether it's of interest or not, but I've written a small library (squirrel) to handle this for genuinely optional modules that a package that I have may require.

The reason I wrote it in the first place was to facilitate using things like coffee-script, stylus, etc in a small build helper tool (rigger) that I wrote without having to specify them as initial dependencies that would be downloaded when the library was first installed. Plugin dependency versions are specified in the package.json file as pluginDependencies, e.g.:

https://github.com/buildjs/rigger/blob/master/package.json#L23

That section in the package.json is optional though as it is just as valid to ask squirrel to get you a specific version when you execute it:

var squirrel = require('squirrel');

squirrel('jade@0.26.x', { allowInstall: true }, function(err, jade) {
});

Regardless of whether you think squirrel is a good fit for your needs here, I'd be very interested in your thoughts on it. Feel free to raise issues against it with issues or thoughts :)

A package may have multiple optional plugins, each which requires it's own set of dependencies. Plugins also might require that you ensure a feature set of the plugin before it can be used.

This level of granularity necessitates configuration of feature sets and dependencies be available on a per plugin basis, and not on a per package basis.

I almost just want to create a package.json file for every broadway plugin, but that seems a bit over-kill. Adding a simple and optional dependencies hash on plugins should solve this.

Will try to get a working solution for this in the next week or so.

So that works as expected and pretty much fixes the issue.

I'm going to integrate some friendly CLI messages to inform the user of what happened.

If we package npm with Broadway, it will be possible to install missing deps on the fly from CLI menu. I really like this idea, but the bloat of adding npm is not really acceptable. Maybe instead of require(npm), we could just cheat and shell out.

Going to think about this for a bit.

Have you considered of lazy installing the modules? I ran in simular issues while working on my front-end build system that has support for a lot of different pre-processors such as stylus, less, sass, coffeescript etc.. So I created a small npm wrapper that installs packages on the fly if they are not found:

https://github.com/3rd-Eden/canihaz
https://github.com/observing/square/blob/master/package.json#L45-59

Implementation example: https://github.com/observing/square/blob/master/plugins/obfuscation.js#L27-34

edit
Oh I just noticed that this was already mentioned above, ignore me ;)

@3rd-Eden - Excellant point.

If we are going to keep Broadway isomorphic, lazy-loaded modules are pretty important ( particularly when lazy-loading javascript assets in the browser ).

I'll change the require statement to a lazy getter, and place the friendly error logic inside there.

This way, in the future, we could implement an async browser script loader using the lazy-getter.