From 119b6b180ffef7e1f2885e117cb125bbc671717b Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Fri, 31 Mar 2017 12:32:08 +0100 Subject: [PATCH 01/10] getComponents-deps now to also return templates and small cade optimizations --- src/cli/domain/get-components-deps.js | 36 ++++++++++++++++++--------- src/cli/facade/dev.js | 12 ++++----- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/cli/domain/get-components-deps.js b/src/cli/domain/get-components-deps.js index bbe513a07..2e9836b44 100644 --- a/src/cli/domain/get-components-deps.js +++ b/src/cli/domain/get-components-deps.js @@ -5,28 +5,40 @@ var path = require('path'); var _ = require('underscore'); module.exports = function(components){ + var deps = { modules: {}, withVersions: {}, templates: {} }; - var deps = { modules: [], withVersions: [] }; - - _.forEach(components, function(c){ + var legacyTemplates = { + 'jade': true, + 'handlebars': true + }; + components.forEach(function(c){ var pkg = fs.readJsonSync(path.join(c, 'package.json')); + var type = pkg.oc.files.template.type; + var dependencies = _.keys(pkg.dependencies); - _.forEach(_.keys(pkg.dependencies), function(d){ + if (!deps.templates[type] && !legacyTemplates[type]) { + deps.templates[type] = true; + } - var version = pkg.dependencies[d], - hasVersion = !_.isEmpty(version), - depToInstall = hasVersion ? (d + '@' + version) : d; + dependencies.forEach(function(name){ + var version = dependencies[name]; + var depToInstall = version.length > 0 + ? (name + '@' + version): name; - if(!_.contains(deps.withVersions, depToInstall)){ - deps.withVersions.push(depToInstall); + if (!deps.withVersions[depToInstall]) { + deps.withVersions[depToInstall] = true; } - if(!_.contains(deps.modules, d)){ - deps.modules.push(d); + if (!deps.modules[name]) { + deps.modules[name] = true; } }); }); - return deps; + return { + modules: Object.keys(deps.modules), + withVersions: Object.keys(deps.withVersions), + templates: Object.keys(deps.templates) + }; }; diff --git a/src/cli/facade/dev.js b/src/cli/facade/dev.js index 636acf793..def542e6e 100644 --- a/src/cli/facade/dev.js +++ b/src/cli/facade/dev.js @@ -106,13 +106,13 @@ module.exports = function(dependencies){ var loadDependencies = function(components, cb){ log.warn(strings.messages.cli.CHECKING_DEPENDENCIES, true); - + var dependencies = getComponentsDependencies(components), missing = getMissingDeps(dependencies.withVersions, components); if(_.isEmpty(missing)){ log.ok('OK'); - return cb(dependencies.modules); + return cb(dependencies); } log.err('FAIL'); @@ -152,11 +152,11 @@ module.exports = function(dependencies){ log.ok('OK'); _.forEach(components, function(component){ logger.log(colors.green('├── ') + component); - }); + }); loadDependencies(components, function(dependencies){ packageComponents(components, function(){ - + var registry = new oc.Registry({ local: true, hotReloading: hotReloading, @@ -167,8 +167,8 @@ module.exports = function(dependencies){ port: port, baseUrl: baseUrl, env: { name: 'local' }, - dependencies: dependencies, - templates: [] + dependencies: dependencies.modules, + templates: dependencies.templates }); registerPlugins(registry); From 12e851841bc60a246cac424d351215ea47719848 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Fri, 31 Mar 2017 12:34:18 +0100 Subject: [PATCH 02/10] wrapped require-template to support requiring from registry --- client/src/get-compiled-template.js | 2 +- client/src/utils/require-template.js | 21 +++++++++++++++------ src/registry/domain/repository.js | 8 ++------ src/utils/require-template.js | 21 +++++++++++++++------ test/unit/registry-domain-repository.js | 2 +- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/client/src/get-compiled-template.js b/client/src/get-compiled-template.js index d01ae288a..31ab4e57c 100644 --- a/client/src/get-compiled-template.js +++ b/client/src/get-compiled-template.js @@ -29,7 +29,7 @@ module.exports = function(cache){ if (type === 'jade') { type = 'oc-template-jade'; } if (type === 'handlebars') { type = 'oc-template-handlebars'; } - var ocTemplate = requireTemplate(type); + var ocTemplate = requireTemplate(type); cb(null, ocTemplate.getCompiledTemplate(templateText, template.key)); }); }; diff --git a/client/src/utils/require-template.js b/client/src/utils/require-template.js index e33eb751a..45aad573f 100644 --- a/client/src/utils/require-template.js +++ b/client/src/utils/require-template.js @@ -1,12 +1,21 @@ 'use strict'; var format = require('stringformat'); +var fs = require('fs'); +var path = require('path'); + var templateNotSupported = 'Error loading component: template "{0}" not supported'; -module.exports = function(type) { - try { - return require(type); - } catch (err) { - throw format(templateNotSupported, type); +module.exports = function(template) { + var localTemplate = path.join(__dirname, '../../', 'node_modules', template); + if (fs.existsSync(localTemplate)) { + return require(localTemplate); + } + + var relativeTemplate = path.resolve('.', 'node_modules', template); + if (fs.existsSync(relativeTemplate)) { + return require(relativeTemplate); } -}; \ No newline at end of file + + throw format(templateNotSupported, template); +}; diff --git a/src/registry/domain/repository.js b/src/registry/domain/repository.js index 5bfe2141d..3171ac681 100644 --- a/src/registry/domain/repository.js +++ b/src/registry/domain/repository.js @@ -12,6 +12,7 @@ var settings = require('../../resources/settings'); var strings = require('../../resources'); var validator = require('./validators'); var versionHandler = require('./version-handler'); +var requireTemplate = require('../../utils/require-template'); module.exports = function(conf){ @@ -26,12 +27,7 @@ module.exports = function(conf){ var coreTemplates = ['oc-template-jade', 'oc-template-handlebars']; var templates = _.union(coreTemplates, conf.templates) .map(function(template){ - var info; - try { - info = require(template).getInfo(); - } catch (err) { - throw new Error(format(strings.errors.registry.TEMPLATE_NOT_FOUND, template)); - } + var info = requireTemplate(template).getInfo(); return { type: info.type, version: info.version, diff --git a/src/utils/require-template.js b/src/utils/require-template.js index 8cf4413b7..d0f436bca 100644 --- a/src/utils/require-template.js +++ b/src/utils/require-template.js @@ -1,12 +1,21 @@ 'use strict'; var format = require('stringformat'); +var path = require('path'); +var fs = require('fs'); + var templateNotSupported = 'Error requiring the template "{0}": oc-template not found'; -module.exports = function(type) { - try { - return require(type); - } catch (err) { - throw format(templateNotSupported, type); +module.exports = function(template) { + var localTemplate = path.join(__dirname, '../../', 'node_modules', template); + if (fs.existsSync(localTemplate)) { + return require(localTemplate); + } + + var relativeTemplate = path.resolve('.', 'node_modules', template); + if (fs.existsSync(relativeTemplate)) { + return require(relativeTemplate); } -}; \ No newline at end of file + + throw format(templateNotSupported, template); +}; diff --git a/test/unit/registry-domain-repository.js b/test/unit/registry-domain-repository.js index 4f00bcc66..e4c5f6f00 100644 --- a/test/unit/registry-domain-repository.js +++ b/test/unit/registry-domain-repository.js @@ -120,7 +120,7 @@ describe('registry : domain : repository', function(){ try { var repository = new Repository(conf); } catch (err) { - expect(err).to.deep.equal(new Error('Template oc-template-react not found')); + expect(err).to.equal('Error requiring the template "oc-template-react": oc-template not found'); } }); }); From dceabcaceb84c42dce74ebea724942fe54a1c401 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Fri, 31 Mar 2017 12:47:38 +0100 Subject: [PATCH 03/10] get-components-deps fix --- src/cli/domain/get-components-deps.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/cli/domain/get-components-deps.js b/src/cli/domain/get-components-deps.js index 2e9836b44..ba80405c8 100644 --- a/src/cli/domain/get-components-deps.js +++ b/src/cli/domain/get-components-deps.js @@ -15,16 +15,17 @@ module.exports = function(components){ components.forEach(function(c){ var pkg = fs.readJsonSync(path.join(c, 'package.json')); var type = pkg.oc.files.template.type; - var dependencies = _.keys(pkg.dependencies); + var dependencies = pkg.dependencies; if (!deps.templates[type] && !legacyTemplates[type]) { deps.templates[type] = true; } - dependencies.forEach(function(name){ + _.keys(dependencies).forEach(function(name){ var version = dependencies[name]; var depToInstall = version.length > 0 - ? (name + '@' + version): name; + ? (name + '@' + version) + : name; if (!deps.withVersions[depToInstall]) { deps.withVersions[depToInstall] = true; @@ -37,8 +38,8 @@ module.exports = function(components){ }); return { - modules: Object.keys(deps.modules), - withVersions: Object.keys(deps.withVersions), - templates: Object.keys(deps.templates) + modules: _.keys(deps.modules), + withVersions: _.keys(deps.withVersions), + templates: _.keys(deps.templates) }; }; From f7ecbafdcb0928a3cacffa469873825450322288 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Fri, 31 Mar 2017 14:46:57 +0100 Subject: [PATCH 04/10] try-catch requiring strategy --- client/src/utils/require-template.js | 23 +++++++++++++++-------- src/utils/require-template.js | 25 ++++++++++++++++--------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/client/src/utils/require-template.js b/client/src/utils/require-template.js index 45aad573f..d186c4ae6 100644 --- a/client/src/utils/require-template.js +++ b/client/src/utils/require-template.js @@ -8,14 +8,21 @@ var templateNotSupported = 'Error loading component: template "{0}" not supporte module.exports = function(template) { var localTemplate = path.join(__dirname, '../../', 'node_modules', template); - if (fs.existsSync(localTemplate)) { - return require(localTemplate); - } - var relativeTemplate = path.resolve('.', 'node_modules', template); - if (fs.existsSync(relativeTemplate)) { - return require(relativeTemplate); - } - throw format(templateNotSupported, template); + try { + if (!!require.cache[localTemplate]) { + delete require.cache[localTemplate]; + } + return require(localTemplate); + } catch(err) { + try { + if (!!require.cache[relativeTemplate]) { + delete require.cache[relativeTemplate]; + } + return require(relativeTemplate); + } catch (err) { + throw format(templateNotSupported, template); + } + } }; diff --git a/src/utils/require-template.js b/src/utils/require-template.js index d0f436bca..a4c536959 100644 --- a/src/utils/require-template.js +++ b/src/utils/require-template.js @@ -6,16 +6,23 @@ var fs = require('fs'); var templateNotSupported = 'Error requiring the template "{0}": oc-template not found'; -module.exports = function(template) { +module.exports = function(template) { var localTemplate = path.join(__dirname, '../../', 'node_modules', template); - if (fs.existsSync(localTemplate)) { - return require(localTemplate); - } - var relativeTemplate = path.resolve('.', 'node_modules', template); - if (fs.existsSync(relativeTemplate)) { - return require(relativeTemplate); - } - throw format(templateNotSupported, template); + try { + if (!!require.cache[localTemplate]) { + delete require.cache[localTemplate]; + } + return require(localTemplate); + } catch(err) { + try { + if (!!require.cache[relativeTemplate]) { + delete require.cache[relativeTemplate]; + } + return require(relativeTemplate); + } catch (err) { + throw format(templateNotSupported, template); + } + } }; From bef1241b16b891b9257d11eae3379d12d3aa9a40 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Tue, 4 Apr 2017 13:29:13 +0100 Subject: [PATCH 05/10] if templatetype is not legacy must be declared as dependency --- src/cli/domain/get-components-deps.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cli/domain/get-components-deps.js b/src/cli/domain/get-components-deps.js index ba80405c8..45ed7330e 100644 --- a/src/cli/domain/get-components-deps.js +++ b/src/cli/domain/get-components-deps.js @@ -18,6 +18,9 @@ module.exports = function(components){ var dependencies = pkg.dependencies; if (!deps.templates[type] && !legacyTemplates[type]) { + if (!dependencies[type]) { + throw new Error(`Template dependency missing. Run "$npm install --save ${type}" to fix it.`) + } deps.templates[type] = true; } From 113910e709443885edef603d6f008ca904383490 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Tue, 4 Apr 2017 14:09:54 +0100 Subject: [PATCH 06/10] try catching requireTemplate within client --- client/src/get-compiled-template.js | 8 +++++++- client/src/settings.js | 3 ++- client/src/template-renderer.js | 8 +++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/client/src/get-compiled-template.js b/client/src/get-compiled-template.js index 31ab4e57c..37002efe3 100644 --- a/client/src/get-compiled-template.js +++ b/client/src/get-compiled-template.js @@ -29,7 +29,13 @@ module.exports = function(cache){ if (type === 'jade') { type = 'oc-template-jade'; } if (type === 'handlebars') { type = 'oc-template-handlebars'; } - var ocTemplate = requireTemplate(type); + var ocTemplate; + try { + ocTemplate = requireTemplate(type); + } catch (e) { + return callback(format(settings.gettingTemplateFailed, type)); + } + cb(null, ocTemplate.getCompiledTemplate(templateText, template.key)); }); }; diff --git a/client/src/settings.js b/client/src/settings.js index 969476c2d..9675ff157 100644 --- a/client/src/settings.js +++ b/client/src/settings.js @@ -13,5 +13,6 @@ module.exports = { registriesEmpty: 'registries must contain at least one endpoint', registriesIsNotObject: 'registries must be an object', serverSideRenderingFail: 'Server-side rendering failed: {0}', - warmupFailed: 'Error warming up oc-client: request {0} failed ({1})' + warmupFailed: 'Error warming up oc-client: request {0} failed ({1})', + gettingTemplateFailed: 'Error requiring template {0}' }; \ No newline at end of file diff --git a/client/src/template-renderer.js b/client/src/template-renderer.js index e495c4b27..993074216 100644 --- a/client/src/template-renderer.js +++ b/client/src/template-renderer.js @@ -13,7 +13,13 @@ module.exports = function(){ if (type === 'jade') { type = 'oc-template-jade'; } if (type === 'handlebars') { type = 'oc-template-handlebars'; } - var ocTemplate = requireTemplate(type); + var ocTemplate; + try { + ocTemplate = requireTemplate(type); + } catch (e) { + return callback(format(settings.gettingTemplateFailed, type)); + } + ocTemplate.render( { template, From 54b8f242f92be0f75fb8341261b2ac344f2708dc Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Tue, 4 Apr 2017 14:14:13 +0100 Subject: [PATCH 07/10] fire error if template is not valid --- src/registry/domain/repository.js | 21 +++++++++++++++------ src/resources/index.js | 6 ++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/registry/domain/repository.js b/src/registry/domain/repository.js index 3171ac681..060e1fdb4 100644 --- a/src/registry/domain/repository.js +++ b/src/registry/domain/repository.js @@ -27,12 +27,21 @@ module.exports = function(conf){ var coreTemplates = ['oc-template-jade', 'oc-template-handlebars']; var templates = _.union(coreTemplates, conf.templates) .map(function(template){ - var info = requireTemplate(template).getInfo(); - return { - type: info.type, - version: info.version, - externals: info.externals - }; + var ocTemplate = requireTemplate(template); + try { + var info = ocTemplate.getInfo(); + return { + type: info.type, + version: info.version, + externals: info.externals + }; + } catch (e) { + throw new Error(format(strings.errors.registry.TEMPLATE_NOT_VALID, template)); + } + + + + }); var local = { diff --git a/src/resources/index.js b/src/resources/index.js index 70be6fb4a..e3ee20704 100644 --- a/src/resources/index.js +++ b/src/resources/index.js @@ -59,7 +59,8 @@ module.exports = { PLUGIN_NOT_IMPLEMENTED: 'registry does not implement plugins: {0}', PLUGIN_NOT_VALID: 'Plugin {0} is not valid', RESOLVING_ERROR: 'component resolving error', - TEMPLATE_NOT_FOUND: 'Template {0} not found' + TEMPLATE_NOT_FOUND: 'Template {0} not found', + TEMPLATE_NOT_VALID: '{0} is not a valid oc-template' }, cli: { COMPONENT_HREF_NOT_FOUND: 'The specified path is not a valid component\'s url', @@ -83,7 +84,8 @@ module.exports = { REGISTRY_NOT_FOUND: 'oc registries not found. Run "oc registry add "', SERVERJS_DEPENDENCY_NOT_DECLARED: 'Missing dependencies from package.json => {0}', TEMPLATE_NOT_FOUND: 'file {0} not found', - TEMPLATE_TYPE_NOT_VALID: 'the template is not valid. Allowed values are handlebars and jade' + TEMPLATE_TYPE_NOT_VALID: 'the template is not valid. Allowed values are handlebars and jade', + TEMPLATE_DEP_MISSING: 'Template dependency missing. Run "$npm install --save {0}" to fix it.' }, generic: 'An error occurred: {0}', s3: { From 12fb30351c7a6a1cf7984dbd4d2b361a00700958 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Tue, 4 Apr 2017 14:15:22 +0100 Subject: [PATCH 08/10] check that in case of oc-template being used, is also declared as dependency in the component pkg --- src/cli/domain/get-components-deps.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cli/domain/get-components-deps.js b/src/cli/domain/get-components-deps.js index 45ed7330e..ca264fb9f 100644 --- a/src/cli/domain/get-components-deps.js +++ b/src/cli/domain/get-components-deps.js @@ -3,6 +3,8 @@ var fs = require('fs-extra'); var path = require('path'); var _ = require('underscore'); +var format = require('stringformat'); +var settings = require('../../resources'); module.exports = function(components){ var deps = { modules: {}, withVersions: {}, templates: {} }; @@ -19,7 +21,7 @@ module.exports = function(components){ if (!deps.templates[type] && !legacyTemplates[type]) { if (!dependencies[type]) { - throw new Error(`Template dependency missing. Run "$npm install --save ${type}" to fix it.`) + throw new Error(format(settings.errors.cli.TEMPLATE_DEP_MISSING, type)); } deps.templates[type] = true; } From 21d79c096730549a06611ed2f3b0c616033ecc39 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Tue, 4 Apr 2017 19:09:02 +0100 Subject: [PATCH 09/10] tests --- client/src/get-compiled-template.js | 4 +- client/src/template-renderer.js | 4 +- client/src/utils/require-template.js | 44 +++++++++++++--- src/cli/domain/package-template.js | 9 +++- src/registry/domain/repository.js | 6 +-- src/registry/routes/helpers/get-component.js | 9 +++- src/utils/require-template.js | 44 +++++++++++++--- test/unit/cli-domain-package-template.js | 2 +- test/unit/registry-domain-repository.js | 2 +- test/unit/utils-require-templates.js | 55 ++++++++++++++++++++ 10 files changed, 151 insertions(+), 28 deletions(-) create mode 100644 test/unit/utils-require-templates.js diff --git a/client/src/get-compiled-template.js b/client/src/get-compiled-template.js index 37002efe3..cc791b225 100644 --- a/client/src/get-compiled-template.js +++ b/client/src/get-compiled-template.js @@ -32,8 +32,8 @@ module.exports = function(cache){ var ocTemplate; try { ocTemplate = requireTemplate(type); - } catch (e) { - return callback(format(settings.gettingTemplateFailed, type)); + } catch (err) { + return callback(err); } cb(null, ocTemplate.getCompiledTemplate(templateText, template.key)); diff --git a/client/src/template-renderer.js b/client/src/template-renderer.js index 993074216..21398d4fa 100644 --- a/client/src/template-renderer.js +++ b/client/src/template-renderer.js @@ -16,8 +16,8 @@ module.exports = function(){ var ocTemplate; try { ocTemplate = requireTemplate(type); - } catch (e) { - return callback(format(settings.gettingTemplateFailed, type)); + } catch (err) { + return callback(err); } ocTemplate.render( diff --git a/client/src/utils/require-template.js b/client/src/utils/require-template.js index d186c4ae6..59b2119fb 100644 --- a/client/src/utils/require-template.js +++ b/client/src/utils/require-template.js @@ -1,28 +1,56 @@ 'use strict'; var format = require('stringformat'); -var fs = require('fs'); var path = require('path'); +var _ = require('underscore'); + +var templateNotFound = 'Error requiring oc-template: "{0}" not found'; +var templateNotValid = 'Error requiring oc-template: "{0}" is not a valid oc-template'; + +function isValidTemplate(template){ + if (typeof template !== 'object') { + return false; + } + + var apiMethods = _.keys(template); + if (apiMethods.length !== 4) { + return false; + } + + return _.contains( + apiMethods, + 'getInfo', + 'getCompiledTemplate', + 'compile', + 'render' + ); +} -var templateNotSupported = 'Error loading component: template "{0}" not supported'; module.exports = function(template) { + var ocTemplate; var localTemplate = path.join(__dirname, '../../', 'node_modules', template); var relativeTemplate = path.resolve('.', 'node_modules', template); - + try { - if (!!require.cache[localTemplate]) { + if (require.cache && !!require.cache[localTemplate]) { delete require.cache[localTemplate]; } - return require(localTemplate); + ocTemplate = require(localTemplate); } catch(err) { try { - if (!!require.cache[relativeTemplate]) { + if (require.cache && !!require.cache[relativeTemplate]) { delete require.cache[relativeTemplate]; } - return require(relativeTemplate); + ocTemplate = require(relativeTemplate); } catch (err) { - throw format(templateNotSupported, template); + throw format(templateNotFound, template); } } + + if (!isValidTemplate(ocTemplate)) { + throw format(templateNotValid, template); + } + + return ocTemplate; }; diff --git a/src/cli/domain/package-template.js b/src/cli/domain/package-template.js index 9baf35480..699ee4e77 100644 --- a/src/cli/domain/package-template.js +++ b/src/cli/domain/package-template.js @@ -16,11 +16,18 @@ var javaScriptizeTemplate = function(functionName, data){ var compileView = function(viewPath, type, cb) { var template = fs.readFileSync(viewPath).toString(); + var ocTemplate; if (type === 'jade') { type = 'oc-template-jade'; } if (type === 'handlebars') { type = 'oc-template-handlebars'; } + + + try { + ocTemplate = requireTemplate(type); + } catch (err) { + return cb(err); + } - var ocTemplate = requireTemplate(type); ocTemplate.compile({ template, viewPath }, function(err, compiledView){ if (err) { return cb(err);} diff --git a/src/registry/domain/repository.js b/src/registry/domain/repository.js index 060e1fdb4..5945d56be 100644 --- a/src/registry/domain/repository.js +++ b/src/registry/domain/repository.js @@ -27,16 +27,16 @@ module.exports = function(conf){ var coreTemplates = ['oc-template-jade', 'oc-template-handlebars']; var templates = _.union(coreTemplates, conf.templates) .map(function(template){ - var ocTemplate = requireTemplate(template); try { + var ocTemplate = requireTemplate(template); var info = ocTemplate.getInfo(); return { type: info.type, version: info.version, externals: info.externals }; - } catch (e) { - throw new Error(format(strings.errors.registry.TEMPLATE_NOT_VALID, template)); + } catch (err) { + throw err; } diff --git a/src/registry/routes/helpers/get-component.js b/src/registry/routes/helpers/get-component.js index 003009f1f..5011510a3 100644 --- a/src/registry/routes/helpers/get-component.js +++ b/src/registry/routes/helpers/get-component.js @@ -229,12 +229,17 @@ module.exports = function(conf, repository){ returnResult(cached); } else { repository.getCompiledView(component.name, component.version, function(err, templateText){ - + var ocTemplate; var type = component.oc.files.template.type; if (type === 'jade') { type = 'oc-template-jade'; } if (type === 'handlebars') { type = 'oc-template-handlebars'; } - var ocTemplate = requireTemplate(type); + try { + ocTemplate = requireTemplate(type); + } catch (err) { + throw err; + } + var template = ocTemplate.getCompiledTemplate(templateText, key); cache.set('file-contents', cacheKey, template); returnResult(template); diff --git a/src/utils/require-template.js b/src/utils/require-template.js index a4c536959..7b735e46d 100644 --- a/src/utils/require-template.js +++ b/src/utils/require-template.js @@ -2,27 +2,55 @@ var format = require('stringformat'); var path = require('path'); -var fs = require('fs'); +var _ = require('underscore'); -var templateNotSupported = 'Error requiring the template "{0}": oc-template not found'; +var templateNotFound = 'Error requiring oc-template: "{0}" not found'; +var templateNotValid = 'Error requiring oc-template: "{0}" is not a valid oc-template'; + +function isValidTemplate(template){ + if (typeof template !== 'object') { + return false; + } + + var apiMethods = _.keys(template); + if (apiMethods.length !== 4) { + return false; + } + + return _.contains( + apiMethods, + 'getInfo', + 'getCompiledTemplate', + 'compile', + 'render' + ); +} module.exports = function(template) { + var ocTemplate; var localTemplate = path.join(__dirname, '../../', 'node_modules', template); var relativeTemplate = path.resolve('.', 'node_modules', template); - + + try { - if (!!require.cache[localTemplate]) { + if (require.cache && !!require.cache[localTemplate]) { delete require.cache[localTemplate]; } - return require(localTemplate); + ocTemplate = require(localTemplate); } catch(err) { try { - if (!!require.cache[relativeTemplate]) { + if (require.cache && !!require.cache[relativeTemplate]) { delete require.cache[relativeTemplate]; } - return require(relativeTemplate); + ocTemplate = require(relativeTemplate); } catch (err) { - throw format(templateNotSupported, template); + throw format(templateNotFound, template); } } + + if (!isValidTemplate(ocTemplate)) { + throw format(templateNotValid, template); + } + + return ocTemplate; }; diff --git a/test/unit/cli-domain-package-template.js b/test/unit/cli-domain-package-template.js index 6323489f0..f3e97b30d 100644 --- a/test/unit/cli-domain-package-template.js +++ b/test/unit/cli-domain-package-template.js @@ -93,7 +93,7 @@ describe('cli : domain : package-template', function(){ }); it('should show error', function(){ - expect(error).to.equal('template.wha compilation failed - Error requiring the template "whazaaa": oc-template not found'); + expect(error).to.equal('template.wha compilation failed - Error requiring oc-template: "whazaaa" not found'); }); }); }); diff --git a/test/unit/registry-domain-repository.js b/test/unit/registry-domain-repository.js index e4c5f6f00..2361f7ce4 100644 --- a/test/unit/registry-domain-repository.js +++ b/test/unit/registry-domain-repository.js @@ -120,7 +120,7 @@ describe('registry : domain : repository', function(){ try { var repository = new Repository(conf); } catch (err) { - expect(err).to.equal('Error requiring the template "oc-template-react": oc-template not found'); + expect(err).to.equal('Error requiring oc-template: "oc-template-react" not found'); } }); }); diff --git a/test/unit/utils-require-templates.js b/test/unit/utils-require-templates.js new file mode 100644 index 000000000..b6a86bad3 --- /dev/null +++ b/test/unit/utils-require-templates.js @@ -0,0 +1,55 @@ +'use strict'; + +var expect = require('chai').expect; +var injectr = require('injectr'); +var sinon = require('sinon'); +var path = require('path'); +var _ = require('underscore'); + + +describe('utils : require-template', function(){ + var globals = { + '__dirname': '.', + }; + + var deps = { + 'path': { + join (a, b, c, template) { + return path.join(a,b,c,template); + }, + resolve (a,b,template) { + var dir = path.join( + path.resolve(), + 'node_modules/oc-template-handlebars/node_modules', + template + ); + return dir; + } + } + }; + + var requireTemplate = injectr( + '../../src/utils/require-template.js', deps, globals + ); + + it('should return the template found if its of the correct type', function(){ + var template = requireTemplate('oc-template-jade'); + var templateAPIs = _.keys(template); + + expect(templateAPIs.length).to.equal(4); + expect(_.contains(templateAPIs, + 'getInfo', + 'getCompiledTemplate', + 'compile', + 'render') + ).to.be.true; + }); + + it('should throw an error if the template found hasn\'t the right format', function(){ + try { + var template = requireTemplate('handlebars'); + } catch (e) { + expect(e).to.equal('Error requiring oc-template: "handlebars" is not a valid oc-template'); + } + }); +}); From ace3bdeb60e46d95879c8b5c15e67eb8dc2f0b87 Mon Sep 17 00:00:00 2001 From: Nick Balestra Date: Tue, 4 Apr 2017 21:08:41 +0100 Subject: [PATCH 10/10] no underscore --- client/src/utils/require-template.js | 13 ++++--------- src/utils/require-template.js | 15 +++++---------- test/unit/utils-require-templates.js | 1 - 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/client/src/utils/require-template.js b/client/src/utils/require-template.js index 59b2119fb..04d309e06 100644 --- a/client/src/utils/require-template.js +++ b/client/src/utils/require-template.js @@ -2,7 +2,6 @@ var format = require('stringformat'); var path = require('path'); -var _ = require('underscore'); var templateNotFound = 'Error requiring oc-template: "{0}" not found'; var templateNotValid = 'Error requiring oc-template: "{0}" is not a valid oc-template'; @@ -12,18 +11,14 @@ function isValidTemplate(template){ return false; } - var apiMethods = _.keys(template); - if (apiMethods.length !== 4) { - return false; - } - - return _.contains( - apiMethods, + return [ 'getInfo', 'getCompiledTemplate', 'compile', 'render' - ); + ].every(function(method){ + return template[method]; + }); } diff --git a/src/utils/require-template.js b/src/utils/require-template.js index 7b735e46d..04d309e06 100644 --- a/src/utils/require-template.js +++ b/src/utils/require-template.js @@ -2,7 +2,6 @@ var format = require('stringformat'); var path = require('path'); -var _ = require('underscore'); var templateNotFound = 'Error requiring oc-template: "{0}" not found'; var templateNotValid = 'Error requiring oc-template: "{0}" is not a valid oc-template'; @@ -12,26 +11,22 @@ function isValidTemplate(template){ return false; } - var apiMethods = _.keys(template); - if (apiMethods.length !== 4) { - return false; - } - - return _.contains( - apiMethods, + return [ 'getInfo', 'getCompiledTemplate', 'compile', 'render' - ); + ].every(function(method){ + return template[method]; + }); } + module.exports = function(template) { var ocTemplate; var localTemplate = path.join(__dirname, '../../', 'node_modules', template); var relativeTemplate = path.resolve('.', 'node_modules', template); - try { if (require.cache && !!require.cache[localTemplate]) { delete require.cache[localTemplate]; diff --git a/test/unit/utils-require-templates.js b/test/unit/utils-require-templates.js index b6a86bad3..3330f93e0 100644 --- a/test/unit/utils-require-templates.js +++ b/test/unit/utils-require-templates.js @@ -36,7 +36,6 @@ describe('utils : require-template', function(){ var template = requireTemplate('oc-template-jade'); var templateAPIs = _.keys(template); - expect(templateAPIs.length).to.equal(4); expect(_.contains(templateAPIs, 'getInfo', 'getCompiledTemplate',