From 23fd554c8f91f38d2e64a885a28b705bb4cb30c2 Mon Sep 17 00:00:00 2001 From: Jon Merrifield Date: Tue, 3 Mar 2015 10:21:24 -0800 Subject: [PATCH 1/6] Allow an instance of templateAdapter to be supplied Allow the consumer to *optionally* supply an actual templateAdapter instance to prevent Rendr from performing a dynamic require. --- shared/app.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/shared/app.js b/shared/app.js index 1cd6a1ef..1894dab5 100644 --- a/shared/app.js +++ b/shared/app.js @@ -56,8 +56,12 @@ module.exports = Backbone.Model.extend({ * We can't use `this.get('templateAdapter')` here because `Backbone.Model`'s * constructor has not yet been called. */ - var templateAdapterModule = attributes.templateAdapter || this.defaults.templateAdapter; - this.templateAdapter = require(templateAdapterModule)({entryPath: entryPath}); + if (attributes.templateAdapterInstance) { + this.templateAdapter = attributes.templateAdapterInstance; + } else { + var templateAdapterModule = attributes.templateAdapter || this.defaults.templateAdapter; + this.templateAdapter = require(templateAdapterModule)({entryPath: entryPath}); + } /** * Instantiate the `Fetcher`, which is used on client and server. From cf3b47bfff8e89c5a73cee37e5af9436a55fb22f Mon Sep 17 00:00:00 2001 From: Jon Merrifield Date: Tue, 3 Mar 2015 13:30:29 -0800 Subject: [PATCH 2/6] Allow ClientRouter class to be supplied directly Allow the calling code to *optionally* supply the ClientRouter class so Rendr doesn't attempt to require it. --- shared/app.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shared/app.js b/shared/app.js index 1894dab5..68fdd490 100644 --- a/shared/app.js +++ b/shared/app.js @@ -6,11 +6,9 @@ var Backbone = require('backbone'), Fetcher = require('./fetcher'), ModelUtils = require('./modelUtils'), - isServer = (typeof window === 'undefined'), - ClientRouter; + isServer = (typeof window === 'undefined'); if (!isServer) { - ClientRouter = require('app/router'); Backbone.$ = window.$ || require('jquery'); } @@ -74,6 +72,8 @@ module.exports = Backbone.Model.extend({ * Initialize the `ClientRouter` on the client-side. */ if (!isServer) { + var ClientRouter = attributes.ClientRouter || require('app/router'); + new ClientRouter({ app: this, entryPath: entryPath, From ea606226b02190fd89ed005929b13ac4a2823bdf Mon Sep 17 00:00:00 2001 From: Jon Merrifield Date: Tue, 3 Mar 2015 17:53:29 -0800 Subject: [PATCH 3/6] Pass instances in options, not attributes, so we don't try and JSON-ify them --- shared/app.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shared/app.js b/shared/app.js index 68fdd490..362f230e 100644 --- a/shared/app.js +++ b/shared/app.js @@ -54,8 +54,8 @@ module.exports = Backbone.Model.extend({ * We can't use `this.get('templateAdapter')` here because `Backbone.Model`'s * constructor has not yet been called. */ - if (attributes.templateAdapterInstance) { - this.templateAdapter = attributes.templateAdapterInstance; + if (options.templateAdapterInstance) { + this.templateAdapter = options.templateAdapterInstance; } else { var templateAdapterModule = attributes.templateAdapter || this.defaults.templateAdapter; this.templateAdapter = require(templateAdapterModule)({entryPath: entryPath}); @@ -72,7 +72,7 @@ module.exports = Backbone.Model.extend({ * Initialize the `ClientRouter` on the client-side. */ if (!isServer) { - var ClientRouter = attributes.ClientRouter || require('app/router'); + var ClientRouter = options.ClientRouter || require('app/router'); new ClientRouter({ app: this, From 07ea9ad4dc5aa5b28b40bea5b3517dbc092f2112 Mon Sep 17 00:00:00 2001 From: Jon Merrifield Date: Mon, 23 Mar 2015 14:26:29 -0800 Subject: [PATCH 4/6] Pull router module name into variable so browserify doesn't follow it --- shared/app.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shared/app.js b/shared/app.js index 362f230e..0fd080ac 100644 --- a/shared/app.js +++ b/shared/app.js @@ -6,7 +6,8 @@ var Backbone = require('backbone'), Fetcher = require('./fetcher'), ModelUtils = require('./modelUtils'), - isServer = (typeof window === 'undefined'); + isServer = (typeof window === 'undefined'), + defaultRouterModule = 'app/router'; if (!isServer) { Backbone.$ = window.$ || require('jquery'); @@ -72,7 +73,7 @@ module.exports = Backbone.Model.extend({ * Initialize the `ClientRouter` on the client-side. */ if (!isServer) { - var ClientRouter = options.ClientRouter || require('app/router'); + var ClientRouter = options.ClientRouter || require(defaultRouterModule); new ClientRouter({ app: this, From 1ae60e7b6f3773334945a61ed497546b2f1bbce0 Mon Sep 17 00:00:00 2001 From: Jon Merrifield Date: Mon, 23 Mar 2015 14:53:36 -0800 Subject: [PATCH 5/6] Fix references to options if not supplied --- shared/app.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/app.js b/shared/app.js index 0fd080ac..7dd4cb77 100644 --- a/shared/app.js +++ b/shared/app.js @@ -55,7 +55,7 @@ module.exports = Backbone.Model.extend({ * We can't use `this.get('templateAdapter')` here because `Backbone.Model`'s * constructor has not yet been called. */ - if (options.templateAdapterInstance) { + if (this.options.templateAdapterInstance) { this.templateAdapter = options.templateAdapterInstance; } else { var templateAdapterModule = attributes.templateAdapter || this.defaults.templateAdapter; @@ -73,7 +73,7 @@ module.exports = Backbone.Model.extend({ * Initialize the `ClientRouter` on the client-side. */ if (!isServer) { - var ClientRouter = options.ClientRouter || require(defaultRouterModule); + var ClientRouter = this.options.ClientRouter || require(defaultRouterModule); new ClientRouter({ app: this, From f9324002778f602071e5cc9879e53abe1263762c Mon Sep 17 00:00:00 2001 From: Jon Merrifield Date: Tue, 24 Mar 2015 11:02:32 -0800 Subject: [PATCH 6/6] Test coverage for new functionality --- test/fixtures/app/template_adapter.js | 6 +++++ test/shared/app.test.js | 37 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 test/fixtures/app/template_adapter.js diff --git a/test/fixtures/app/template_adapter.js b/test/fixtures/app/template_adapter.js new file mode 100644 index 00000000..0b5e0a00 --- /dev/null +++ b/test/fixtures/app/template_adapter.js @@ -0,0 +1,6 @@ +module.exports = function (options) { + return { + name: 'Test template adapter', + suppliedOptions: options + }; +} diff --git a/test/shared/app.test.js b/test/shared/app.test.js index 4e973838..23527e63 100644 --- a/test/shared/app.test.js +++ b/test/shared/app.test.js @@ -26,4 +26,41 @@ describe('BaseApp', function() { new MyApp(); }); }); + + describe('constructor', function() { + context('with a custom templateAdapter module name', function() { + beforeEach(function () { + this.attributes = {templateAdapter: '../test/fixtures/app/template_adapter'}; + }); + + it('creates the templateAdapter we specify', function() { + var app = new App(this.attributes); + + expect(app.templateAdapter).to.have.property('name', 'Test template adapter'); + }); + + it('supplies the entryPath to the template adapter', function() { + var app = new App(this.attributes, {entryPath: 'myEntryPath'}); + + expect(app.templateAdapter).to.have.deep.property('suppliedOptions.entryPath', 'myEntryPath'); + }); + }); + + context('with a concrete templateAdapterInstance', function() { + it('uses the supplied templateAdapterInstance', function() { + var myTemplateAdapter = {}; + var app = new App(null, {templateAdapterInstance: myTemplateAdapter}); + + expect(app.templateAdapter).to.equal(myTemplateAdapter); + }); + + it('does not try to require a template adapter by name', function () { + new App({ + templateAdapter: 'non existent module name - should throw' + }, { + templateAdapterInstance: {} + }); + }); + }); + }); });