App Boilerplate: filters ?

[ErisDS]

This is intended as a discussion point.

Should filters be baked into the app boilerplate?

https://github.com/jgable/ghost-plugin/blob/master/lib/FilterPlugin.js

The idea ends up being much like defining events in Backbone. It's definitely a pretty awesome dev experience, and it makes a lot of sense. It looks like:

MyApp = App.extend({
    filters: {
      filterName: someFunction
    },

    someFunction: function (data) {
        // do something with data
        return data;
    }
});

Alternatively, should we have extend the base app boilerplate, and have a filter app boilerplate as in @jgable's original examples? I think filters are going to be common enough to warrant this neat syntax always being available.

Thoughts?

[jgable]

Including the filter stuff in the base App gives people a good amount of power right out of the gate, but could make for a steeper learning curve if they don't know they have to do (or are intimidated by doing) App.prototype.activate.apply(this, arguments) if they ever implement their own activate method.

Also, I can't really think of a situation where a plugin wouldn't at least need to register one filter to be useful (maybe later with platform plugins, then again those could also use filters).

My Vote: include them, but we should provide examples that clearly show and educate about calling supers like App.prototype.activate.apply(this, arguments) when overriding a method.

[ErisDS]

Including the filter stuff in the base App gives people a good amount of power right out of the gate

👍 totally agree

but could make for a steeper learning curve if they don't know they have to [call super]

This is a very interesting point that I hadn't thought about.

App.prototype.activate.apply(this, arguments) is complicated and very very likely to cause trouble. The fundamental problem, in my mind, is that the point of .extend is to hide complexities away, but knowing to call the super requires that you know what methods the super has, and that you are overriding them, and therefore you're back to having to know about some of the complexities we're trying to hide.. if you get what I mean?

Anyway, I think it would be great if we could work around it somehow?

A couple of ideas:

Provide a 5th life-cycle function perhaps called init, which only the ghost-app boilerplates make use of, they know to call that function if they extend each other - Ghost core can be smart enough to run init, then activate. This is a reasonably simple solution but a bit dirty in terms of muddying the clear waters of the lifecycle.

Change extend such that, if it finds an overridden lifecycle method, it wraps that method in another method which calls the method, and then calls the super. This is possible I think, it's a bit dirty in terms of being magical, but it does keep all the complexity in the extend method.

Whaddyathink?

[jgable]

We actually already have an initialize method that is called after the constructor and before any life cycle methods.

We could go all java-like and expose things like onInstall, onActivate that are called after the base implementations.

App.prototype.activate = function () {
    // register filters in filters: { ... }

    return this.onActivate();
};

// No-op by default for onActivate
App.prototype.onActivate = function () { return; }

Makes me feel kind of gross, but it would probably be the most straight forward way to provide extension without calling super.

[simonexmachina]

App.prototype.activate.apply(this, arguments) is complicated and very very likely to cause trouble.

This is why I think an event-based pattern is better, users can add/remove listeners as they like, and they don't need to call super to avoid breaking things. We can always use an event library that provides the ability to set this e.g.

ghost.filters.on('foo', this, function(ev) {
  // do something with data
  ev.data = this.doSomething(ev.data);
});

Another possibility is to use a middleware-style next function e.g.

MyApp = App.extend({
    filters: {
      filterName: someFunction
    },

    someFunction: function (data, next) {
        // do something with data
        data = this.doSomething(ev.data)
        next(data);
    }
});

But I think I prefer the first form, it feels more idiomatic, less boilerplate, more familiar to web devs.

[jgable]

Our filters already behave like the first example you have, except they
return the new value instead of just modifying it.

One other thing that filters allow is setting a priority, which I don't
think an event based approach would provide.

[simonexmachina]

Okay @jgable sorry I was referring to the super problem using filters as a (poor) example. Here's another way of putting it:

App.prototype.init = function() {
  GhostApp.prototype.init.apply(this, arguments);
  this.on('activate', this, function() {
    console.log('activated');
  });
});

We could even make this simpler by adding the following to the superclass:

GhostApp.prototype.hooks = {
  'activate': 'onActivate'
};
GhostApp.prototype.init = function() {
  for( var event in this.hooks ) {
    var method = this.hooks[event];
    if( this[method] ) {
      this.on(event, this, this[method]);
    }
  }
};

This would allow us to simplify the plugin code to:

App.prototype.onActivate = function() {
  console.log('activated');
};

[ErisDS]

I think we want to have a promise chain, not events, so we can be sure when things are complete.

[hswolff]

Another possible solution is to have Ghost manage calling the Ghost-app super method.

i.e. if you have an app:

var ExampleApp = {
  'filter': function() {
    // do filter stuff
  }
};

Ghost can do something like:

var ExampleApp = require('example-app');
var GhostApp = require('ghost-app');

var runFilters = function() {
   GhostApp.filter.call(ExampleApp);
   ExampleApp.filter();
};

[ErisDS]

With #8, what we get is the ability to do the following:

var App = require('example-app'),
      MyApp;

MyApp = App.extend({
    filters: {
        'ghost_head': [6, 'handleGhostHead'],
        'ghost_foot': 'handleGhostFoot',
    },
    handeGhostHead: function () {},
    handeGhostFoot: function () {}
});

I think this is super neat, and it's done so it doesn't interfere with the lifecycle.

The only issue I have now is that it is also perfectly possible to do:

MyApp = App.extend({
    activate: function () {
        this.app.filter.register('ghost_head', this.handleGhostHead);
    },
    handeGhostHead: function () {}
});

This breaks our 'one clear way to do something'. But I think that both syntaxes are useful. The simplified filter syntax is for basic apps that just want to do a thing with a filter. The advanced syntax means it is possible to register and unregister filters at specific times / in particular cases. Any thoughts?

Next question is does the same syntax make sense for helpers?

[jgable]

I don't think it's going to be possible to keep the advanced case from
happening since they will have access to the "firehose", so to speak, at
all times through this.app.

The helpers can be done in the same way. I have another class set up like
that in my old ghost plugin repo. We'd probably just add that to the same
GhostApp class here instead of another class.