TryGhost / Ghost-App

Includes for Ghost Apps
MIT License
111 stars 39 forks source link

App Boilerplate: helpers #14

Closed ErisDS closed 10 years ago

ErisDS commented 10 years ago

Want to leave this open whilst we're building out example apps and getting the platform into a usable state.

We have the backbone-events style JSON syntax for filters, would it make sense to add this for helpers too?

I'm referring to the ability to do:

MyApp = App.extend({
    helpers: {
       myHelper: 'myHelper'
    },
    myHelper: function () {},
});

I'm not sure if the same syntax makes sense, because the name of the helper is going to be the same as the name of the function. Also, how would we distinguish between async and non-async helpers?

jgable commented 10 years ago

I think we've got a couple options here:

  1. have an asyncHelpers and helpers hash (kinda not very "one way to do things")
  2. have async helpers use a special naming convention like myHelper: 'asyncMyHelper'
  3. have all helpers be async and we just wrap them in a when.resolve(..) when registering

I think 3 is easiest to implement and develop against so I vote for it. 2 would be my second choice.

AJ-Acevedo commented 10 years ago

Option 3 sounds ideal. What would be the disadvantage (if any) of going with option 3?

jgable commented 10 years ago

The only thing I can come up with is that it's a small, maybe even tiny, performance penalty to have to wrap everything in when.resolve.

ErisDS commented 10 years ago

@jgable the other downside is that async helpers are a creation of express-hbs, and unfortunately at present don't work with subexpressions - having everything be async could potentially result in some unexpected results.

I'm not particularly sold that this is even a good idea. It makes sense for filters because they are such a fundamental part of apps - but the more I think about this the less I think it makes sense.

I get the feeling it's a pre-optimisation, and that if we see a need for it in future we could add it?

Would love to hear what other people's feelings are on it though.

jgable commented 10 years ago

@ErisDS I agree about the reliance on a non-standard handlebars feature from express-hbs.

On the pre-optimization (yeah, that's a z in there) front, I'd also agree. None of the ideas I've had for apps have required extra helpers yet.

ErisDS commented 10 years ago

to-may-to to-mah-to :dancers:

Let's leave this then, I'll close it unless someone else comes up with a different perspective :)

k-j-kleist commented 9 years ago

Has somebody actually created a handlebars helper using an app?

https://github.com/TryGhost/Ghost/blob/master/core/server/apps/proxy.js#L62 seems to indicate it should be possible. However this code does not work:

'use strict';

var App = require('ghost-app');
var MyApp;

MyApp = App.extend({

    initialize: function () {
        console.log('MyApp: initialize()');
    },

    install: function () {
        console.log('MyApp: install()');
    },

    uninstall: function () {
        console.log('MyApp: uninstall()');
    },

    activate: function () {
        console.log('MyApp: activate()');
    },

    deactivate: function () {
        console.log('MyApp: deactivate()');
        },

    // https://github.com/TryGhost/Ghost/blob/master/core/server/apps/proxy.js#L62

    // "package.json":
    //
    // "ghost": {
    //    "permissions": {
    //        "helpers": ["fb_post_url"]
    //    }
    // }

    helpers: {
        fb_post_url: 'getFbPostUrl'
    },

    getFbPostUrl: function(fb_post_url) {
            return('http://example.com/foo/bar/baz');
    }

});

module.exports = MyApp;

I'd hate to abuse config.js as often suggested by posts like this:

http://zackehh.com/safely-creating-custom-handlebars-helpers/

ErisDS commented 9 years ago

This issue was closed because this version of the syntax was proposed and then rejected ;)

Therefore the way to register a helper is via the app proxy object as part of the activate function:

activate: function () {
    console.log('MyApp: activate()');
    this.app.helpers.register('fb_post_url', this.getFbPostUrl);
}

Prolly worth adding that to the docs if you have a second :+1:

k-j-kleist commented 9 years ago

Works like a charm (using "this.ghost" instead of "this.app"), thanks!

I've updated https://github.com/TryGhost/Ghost/wiki/Apps-Getting-Started-for-Ghost-Devs accordingly.