HTML helpers refactor - issue #246 items 2 and 5.

- moved template logic out of individual helpers and into Ghost
- simplified template-driven helpers into closures which maintain the context of handlebars
- with handlebars context we have access to data, so don't need to pass data in
- check data to test that it is a simple object and not a function
- moved helpers back into index.js
- provided tests for both template functions in ghost and the nav helper so we are back to where we were

core/frontend/helpers/index.js

@@ -1,12 +1,14 @@
 var _ = require('underscore'),
     moment = require('moment'),
     when = require('when'),
-    pagination = require('./paginate'),
-    navHelper = require('./navigation'),
     hbs = require('express-hbs'),
+    errors = require('../../shared/errorHandling'),
     coreHelpers;
 
 coreHelpers = function (ghost) {
+    var navHelper,
+        paginationHelper;
+
     /**
      * [ description]
      * @todo ghost core helpers + a way for themes to register them
@@ -119,10 +121,40 @@ coreHelpers = function (ghost) {
         }
         return ret;
     });
-    // Just one async helper for now, but could be more in the future
+
+    // ## Template driven helpers
+    // Template driven helpers require that their template is loaded before they can be registered.
+
+    // ###Nav Helper
+    // `{{nav}}`
+    // Outputs a navigation menu built from items in config.js
+    navHelper = ghost.loadTemplate('nav').then(function (templateFn) {
+        ghost.registerThemeHelper('nav', function (options) {
+            if (!_.isObject(this.navItems) || _.isFunction(this.navItems)) {
+                errors.logAndThrowError('navItems data is not an object or is a function');
+                return;
+            }
+            return new hbs.handlebars.SafeString(templateFn({links: this.navItems}));
+        });
+    });
+
+    // ### Pagination Helper
+    // `{{paginate}}`
+    // Outputs previous and next buttons, along with info about the current page
+    paginationHelper = ghost.loadTemplate('pagination').then(function (templateFn) {
+        ghost.registerThemeHelper('paginate', function (options) {
+            if (!_.isObject(this.pagination) || _.isFunction(this.pagination)) {
+                errors.logAndThrowError('pagination data is not an object or is a function');
+                return;
+            }
+            return new hbs.handlebars.SafeString(templateFn(this.pagination));
+        });
+    });
+
+    // Return once the template-driven helpers have loaded
     return when.join(
-        navHelper.registerWithGhost(ghost),
+        navHelper,
-        pagination.registerWithGhost(ghost)
+        paginationHelper
     );
 };
 

core/frontend/helpers/navigation.js

@@ -1,46 +0,0 @@
-var fs = require('fs'),
-    path = require('path'),
-    _ = require('underscore'),
-    handlebars = require('express-hbs').handlebars,
-    nodefn = require('when/node/function'),
-
-    NavHelper;
-
-NavHelper = function (navTemplate) {
-    // Bind the context for our methods.
-    _.bindAll(this, 'compileTemplate', 'renderNavItems');
-
-    if (_.isFunction(navTemplate)) {
-        this.navTemplateFunc = navTemplate;
-    } else {
-        this.navTemplatePath = navTemplate;
-    }
-};
-
-NavHelper.prototype.compileTemplate = function (templatePath) {
-    var self = this;
-
-    // Allow people to overwrite the navTemplatePath
-    templatePath = templatePath || this.navTemplatePath;
-
-    return nodefn.call(fs.readFile, templatePath).then(function (navTemplateContents) {
-        // TODO: Can handlebars compile async?
-        self.navTemplateFunc = handlebars.compile(navTemplateContents.toString());
-    });
-};
-
-NavHelper.prototype.renderNavItems = function (navItems) {
-    return new handlebars.SafeString(this.navTemplateFunc({links: navItems}));
-};
-
-// A static helper method for registering with ghost
-NavHelper.registerWithGhost = function (ghost) {
-    var templatePath = path.join(ghost.paths().frontendViews, 'nav.hbs'),
-        ghostNavHelper = new NavHelper(templatePath);
-
-    return ghostNavHelper.compileTemplate().then(function () {
-        ghost.registerThemeHelper("nav", ghostNavHelper.renderNavItems);
-    });
-};
-
-module.exports = NavHelper;

core/frontend/helpers/paginate.js

@@ -1,45 +0,0 @@
-var fs = require('fs'),
-    path = require('path'),
-    _ = require('underscore'),
-    handlebars = require('express-hbs').handlebars,
-    nodefn = require('when/node/function'),
-
-    PaginationHelper;
-
-PaginationHelper = function (paginationTemplate) {
-    // Bind the context for our methods.
-    _.bindAll(this, 'compileTemplate', 'renderPagination');
-
-    if (_.isFunction(paginationTemplate)) {
-        this.paginationTemplateFunc = paginationTemplate;
-    } else {
-        this.paginationTemplatePath = paginationTemplate;
-    }
-};
-
-PaginationHelper.prototype.compileTemplate = function (templatePath) {
-    var self = this;
-
-    // Allow people to overwrite the paginationTemplatePath
-    templatePath = templatePath || this.paginationTemplatePath;
-
-    return nodefn.call(fs.readFile, templatePath).then(function (paginationContents) {
-        // TODO: Can handlebars compile async?
-        self.paginationTemplateFunc = handlebars.compile(paginationContents.toString());
-    });
-};
-
-PaginationHelper.prototype.renderPagination = function (context) {
-    return new handlebars.SafeString(this.paginationTemplateFunc(context));
-};
-
-PaginationHelper.registerWithGhost = function (ghost) {
-    var templatePath = path.join(ghost.paths().frontendViews, 'pagination.hbs'),
-        paginationHelper = new PaginationHelper(templatePath);
-
-    return paginationHelper.compileTemplate().then(function () {
-        ghost.registerThemeHelper("paginate", paginationHelper.renderPagination);
-    });
-};
-
-module.exports = PaginationHelper;

core/ghost.js

@@ -6,8 +6,10 @@ var config = require('./../config'),
     when = require('when'),
     express = require('express'),
     errors = require('../core/shared/errorHandling'),
+    fs = require('fs'),
     path = require('path'),
     hbs = require('express-hbs'),
+    nodefn = require('when/node/function'),
     _ = require('underscore'),
     Polyglot = require('node-polyglot'),
 
@@ -167,6 +169,21 @@ Ghost.prototype.updateSettingsCache = function (settings) {
     }
 };
 
+// ## Template utils
+
+Ghost.prototype.compileTemplate = function (templatePath) {
+    return nodefn.call(fs.readFile, templatePath).then(function (templateContents) {
+        return hbs.handlebars.compile(templateContents.toString());
+    }, errors.logAndThrowError);
+};
+
+Ghost.prototype.loadTemplate = function (name) {
+    // TODO: allow themes to override these templates
+    var templatePath = path.join(this.paths().frontendViews, name + '.hbs');
+
+    return this.compileTemplate(templatePath);
+};
+
 /**
  * @param  {string}   name
  * @param  {Function} fn

core/test/ghost/fixtures/test.hbs

@@ -0,0 +1 @@
+<h1>HelloWorld</h1>

core/test/ghost/frontend_helpers_index_spec.js

@@ -0,0 +1,55 @@
+/*globals describe, beforeEach, it*/
+var should = require('should'),
+    sinon = require('sinon'),
+    when = require('when'),
+    _ = require('underscore'),
+    handlebars = require('express-hbs').handlebars,
+    path = require('path'),
+    helpers = require('../../frontend/helpers'),
+    Ghost = require('../../ghost');
+
+describe('Core Helpers', function () {
+
+    var ghost;
+
+    beforeEach(function () {
+        ghost = new Ghost();
+    });
+
+
+    describe('Navigation Helper', function () {
+
+        it('can render nav items', function (done) {
+            var templateSpy = sinon.spy(function (data) { return "rendered " + data.links.length; }),
+                compileSpy = sinon.stub(ghost, 'compileTemplate').returns(when.resolve(templateSpy)),
+                fakeNavItems = [{
+                    title: 'test1',
+                    url: '/test1'
+                }, {
+                    title: 'test2',
+                    url: '/test2'
+                }],
+                rendered;
+
+            helpers.loadCoreHelpers(ghost).then(function () {
+
+                should.exist(handlebars.helpers.nav);
+
+                rendered = handlebars.helpers.nav.call({navItems: fakeNavItems});
+
+                // Returns a string returned from navTemplateFunc
+                should.exist(rendered);
+                rendered.string.should.equal("rendered 2");
+
+                compileSpy.called.should.equal(true);
+                templateSpy.called.should.equal(true);
+                templateSpy.calledWith({ links: fakeNavItems }).should.equal(true);
+
+
+                compileSpy.restore();
+
+                done();
+            }, done);
+        });
+    });
+});

core/test/ghost/frontend_helpers_navigation_spec.js

@@ -1,65 +0,0 @@
-/*globals describe, beforeEach, it*/
-var should = require('should'),
-    sinon = require('sinon'),
-    _ = require('underscore'),
-    path = require('path'),
-    NavHelper = require('../../frontend/helpers/navigation');
-
-describe('Navigation Helper', function () {
-    var navTemplatePath = path.join(process.cwd(), 'core/frontend/views/nav.hbs');
-
-    should.exist(NavHelper, "Navigation helper exists");
-
-    it('can compile the nav template', function (done) {
-        var helper = new NavHelper(navTemplatePath);
-
-        helper.compileTemplate().then(function () {
-            should.exist(helper.navTemplateFunc);
-            _.isFunction(helper.navTemplateFunc).should.equal(true);
-
-            done();
-        }, done);
-    });
-
-    it('can render nav items', function () {
-        var helper = new NavHelper(function (data) { return "rendered " + data.links.length; }),
-            templateSpy = sinon.spy(helper, 'navTemplateFunc'),
-            fakeNavItems = [{
-                title: 'test1',
-                url: '/test1'
-            }, {
-                title: 'test2',
-                url: '/test2'
-            }],
-            rendered;
-
-        rendered = helper.renderNavItems(fakeNavItems);
-
-        // Returns a string returned from navTemplateFunc
-        should.exist(rendered);
-        rendered.string.should.equal("rendered 2");
-
-        templateSpy.calledWith({ links: fakeNavItems }).should.equal(true);
-    });
-
-    it('can register with ghost', function (done) {
-        var fakeGhost = {
-                paths: function () {
-                    return {
-                        frontendViews: path.join(process.cwd(), 'core/frontend/views/')
-                    };
-                },
-
-                registerThemeHelper: function () {
-                    return;
-                }
-            },
-            registerStub = sinon.stub(fakeGhost, 'registerThemeHelper');
-
-        NavHelper.registerWithGhost(fakeGhost).then(function () {
-            registerStub.called.should.equal(true);
-
-            done();
-        }, done);
-    });
-});

core/test/ghost/ghost_spec.js

@@ -2,9 +2,17 @@
 var should = require('should'),
     when = require('when'),
     sinon = require('sinon'),
+    path = require('path'),
+    _ = require('underscore'),
     Ghost = require('../../ghost');
 
 describe("Ghost API", function () {
+    var testTemplatePath = 'core/test/ghost/fixtures/',
+        ghost;
+
+    beforeEach(function () {
+        ghost = new Ghost();
+    });
 
     it("is a singleton", function () {
         var logStub = sinon.stub(console, "log"),
@@ -16,8 +24,7 @@ describe("Ghost API", function () {
     });
 
     it("uses init() to initialize", function (done) {
-        var ghost = new Ghost(),
+        var fakeDataProvider = {
-            fakeDataProvider = {
                 init: function () {
                     return when.resolve();
                 }
@@ -43,8 +50,7 @@ describe("Ghost API", function () {
     });
 
     it("can register filters with specific priority", function () {
-        var ghost = new Ghost(),
+        var filterName = 'test',
-            filterName = 'test',
             filterPriority = 9,
             testFilterHandler = sinon.spy();
 
@@ -57,8 +63,7 @@ describe("Ghost API", function () {
     });
 
     it("can register filters with default priority", function () {
-        var ghost = new Ghost(),
+        var filterName = 'test',
-            filterName = 'test',
             defaultPriority = 5,
             testFilterHandler = sinon.spy();
 
@@ -71,8 +76,7 @@ describe("Ghost API", function () {
     });
 
     it("executes filters in priority order", function (done) {
-        var ghost = new Ghost(),
+        var filterName = 'testpriority',
-            filterName = 'testpriority',
             testFilterHandler1 = sinon.spy(),
             testFilterHandler2 = sinon.spy(),
             testFilterHandler3 = sinon.spy();
@@ -91,4 +95,44 @@ describe("Ghost API", function () {
             done();
         });
     });
+
+    it("can compile a template", function (done) {
+        var template = path.join(process.cwd(), testTemplatePath, 'test.hbs');
+
+        should.exist(ghost.compileTemplate, 'Template Compiler exists');
+
+        ghost.compileTemplate(template).then(function (templateFn) {
+            should.exist(templateFn);
+            _.isFunction(templateFn).should.equal(true);
+
+            templateFn().should.equal('<h1>HelloWorld</h1>');
+            done();
+        }, done);
+    });
+
+    it("loads templates for helpers", function (done) {
+        var compileSpy = sinon.spy(ghost, 'compileTemplate');
+
+        should.exist(ghost.loadTemplate, 'load template function exists');
+
+        // In order for the test to work, need to replace the path to the template
+        ghost.paths = sinon.stub().returns({
+            frontendViews: path.join(process.cwd(), testTemplatePath)
+        });
+
+        ghost.loadTemplate('test').then(function (templateFn) {
+            // test that compileTemplate was called with the expected path
+            compileSpy.calledOnce.should.equal(true);
+            compileSpy.calledWith(path.join(process.cwd(), testTemplatePath, 'test.hbs')).should.equal(true);
+
+            should.exist(templateFn);
+            _.isFunction(templateFn).should.equal(true);
+
+            templateFn().should.equal('<h1>HelloWorld</h1>');
+
+            compileSpy.restore();
+
+            done();
+        }, done);
+    });
 });