-
Notifications
You must be signed in to change notification settings - Fork 12
Changes to become v1.x #30
Changes from 37 commits
c50c6f9
62624e3
68aae66
0b18d7d
400efff
1b92a95
0f440b1
efa64d0
71e879a
e30dfea
6a8d836
a35d02b
5e125f7
312c099
055be18
8b666b5
491847b
c34d1d1
fa385dd
036e3b0
c0ec0b6
eb6b5cb
11dc7f0
5657129
082ea49
d41cfab
422073f
5c5cb94
f224899
a03afac
985f297
0d1f0a1
528b2c1
7c77448
206295a
7e8422c
c600fda
457e310
9c8b299
5af265a
6f99310
0a7d7c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,3 +2,6 @@ | |
| node_modules/ | ||
| coverage/ | ||
| npm-debug.log | ||
| *.swp | ||
| test/fixtures/tmp/ | ||
| nyc_output/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,88 +1,93 @@ | ||
| engine-munger | ||
| engine-munger | ||
| ============= | ||
|
|
||
| Lead Maintainer: [Aria Stewart](https://github.com/aredridel) | ||
| A replacement Express view class that provides asynchronous resolution, allows | ||
| engines to use the lookup method to locate partials, and extends the lookup | ||
| method to be configurable based on i18n locale and a template specialization | ||
| rule map. | ||
|
|
||
| This is a backport of [work for Express 5](https://github.com/strongloop/express/pull/2653) | ||
|
|
||
| Lead Maintainer: [Aria Stewart](https://github.com/aredridel) | ||
|
|
||
| [](https://travis-ci.org/krakenjs/engine-munger) | ||
|
|
||
| A template engine munging library. | ||
| It looks for appropriate template consolidation library for specified view engine and includes i18n and specialization in the workflow. | ||
| What does i18n mean? | ||
| -------------------- | ||
|
|
||
| i18n means "internationalization". Given a `locale` property in the render options, `engine-munger` will look for content in a locale-specific directory (or in a fallback locale if that is not a match) for templates and partials. This is particularly useful with template engines that pre-localize their compiled forms, such as with [`localizr`](https://github.com/krakenjs/localizr) and [`dustjs-linkedin`](http://dustjs.com/) together. | ||
|
|
||
| Note: If you use specialization features, you must use dustjs before 2.6.0. | ||
| What does specialization mean? | ||
| ------------------------------ | ||
|
|
||
| ###### What does i18n mean ? | ||
| Localization of included content tags for a specified locale. Currently supported only for dust templating engine and internally uses the module [localizr](https://github.com/krakenjs/localizr) for translating content tags included in the templates | ||
| Ability to switch a specific template with another based on a rule set specified in the app config. The actual rule parsing is done using the module [`karka`](https://github.com/krakenjs/karka). | ||
|
|
||
| ###### What does specialization mean ? | ||
| Ability to switch a specific template with another based on a rule set specified in the app config. The actual rule parsing is done using the module [karka](https://github.com/krakenjs/karka) and can be extended and used in any templating engine and not dust. | ||
| All engine-munger does is includes a specialization map with the list of key value pairs using the karka module. | ||
|
|
||
| ```javascript | ||
| { | ||
| _specialization : { | ||
| ... | ||
| originalTemplate : <mappedTemplate> | ||
| ... | ||
| specialization : { | ||
| 'jekyll': [ | ||
| { | ||
| is: 'hyde', | ||
| when: { | ||
| 'whoAmI': 'badGuy' | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ##### Currently supported template engines out of the box: | ||
|
|
||
| * Dust: Engine types 'js' and 'dust' | ||
| The above will switch the template from `jekyll` to `hyde` if the render options contain `"whoAmI": "badGuy"`. Rules can be as complex as you need for your application and are particularly good for running A/B tests. | ||
|
|
||
| Using engine-munger in an application | ||
| ===================================== | ||
|
|
||
| Simple Usage: | ||
| This example uses the [`adaro`](https://github.com/krakenjs/adaro) template engine, which wraps dust up as an express view engine, and uses engine-munger's more sophisticated lookup method to find partials, allowing them to be localized and specialized based on the render options. | ||
|
|
||
| ```javascript | ||
| var engine-munger = require('engine-munger'), | ||
| app = require('express')(); | ||
| var munger = require('engine-munger'); | ||
| var adaro = require('adaro'); | ||
| var app = require('express')(); | ||
|
|
||
| var specialization = { | ||
| 'jekyll': [ | ||
| { | ||
| is: 'hyde', | ||
| when: { | ||
| 'whoAmI': 'badGuy' | ||
| } | ||
| } | ||
| ] | ||
| }; | ||
| app.set("view", munger.makeViewClass({ | ||
| "dust": { | ||
| specialization: specialization | ||
| }, | ||
| "js": { | ||
| specialization: specialization, | ||
| i18n: { | ||
| fallback: 'en-US', | ||
| contentPath: 'locales' | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| app.engine('dust', engine-munger['dust'](settings, config)); | ||
| app.engine('js', engine-munger['js'](settings, config)); | ||
| ``` | ||
| var engineConfig = {}; // Configuration for your view engine | ||
|
|
||
| * settings : [JSON] Arguments you want passed to the templating engine, | ||
| * config: [JSON] used to specify whether you need i18n/specialization enabled. It also compulsarily requires the 'view' and 'view engine' settings passed into express app. | ||
|
|
||
| If you are using kraken-js 1.0 along with engine-munger, the kraken generator will automatically set this all up for you. | ||
| But if you want to use this with a stand alone express app with dust as templating engine, you can specify as follows: | ||
|
|
||
| Example params: | ||
|
|
||
| ```javascript | ||
| var settings = {cache: false}, | ||
| config = { | ||
| 'views': 'public/templates', | ||
| 'view engine': 'dust', | ||
| 'i18n': { | ||
| 'fallback': 'en-US', | ||
| 'contentPath': 'locales' | ||
| }, | ||
| specialization: { | ||
| 'jekyll': [ | ||
| { | ||
| is: 'hyde', | ||
| when: { | ||
| 'whoAmI': 'badGuy' | ||
| } | ||
| } | ||
| ] | ||
|
|
||
| } | ||
| }; | ||
| ``` | ||
| app.engine('dust', adaro.dust(engineConfig)); | ||
| app.engine('js', adaro.js(engineConfig)); | ||
| ``` | ||
|
|
||
| Running Tests: | ||
|
|
||
| ```shell | ||
| npm test | ||
| ``` | ||
| To run tests: | ||
| $ npm test | ||
|
|
||
| To run coverage | ||
| $ npm run-script cover | ||
| To run coverage: | ||
|
|
||
| To run lint | ||
| $ npm run-script lint | ||
| ```shell | ||
| npm run cover | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to run
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, run has always been an alias for run-script. What happened when you did
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh. sorry. i was simply using
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome coverage metrics btw!! |
||
| ``` | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,51 +15,198 @@ | |
| │ See the License for the specific language governing permissions and │ | ||
| │ limitations under the License. │ | ||
| \*───────────────────────────────────────────────────────────────────────────*/ | ||
| "use strict"; | ||
| var path = require('path'); | ||
| var debug = require('debuglog')('engine-munger'); | ||
| var fs = require('fs'); | ||
| var permutron = require('permutron'); | ||
| var oldView = require('express/lib/view'); | ||
| var karka = require('karka'); | ||
| var aproba = require('aproba'); | ||
| var bcp47 = require('bcp47'); | ||
| var bcp47stringify = require('bcp47-stringify'); | ||
|
|
||
| 'use strict'; | ||
| var engine = require('adaro'), | ||
| munger = require('./lib/munger'); | ||
| /** | ||
| * Make a View class that uses our configuration, set far in advance of | ||
| * instantiation because Express passes very little to the actual constructor. | ||
| */ | ||
| function makeViewClass(config) { | ||
| aproba('O', arguments); | ||
|
|
||
| var conf = normalizeConfigs(config); | ||
|
|
||
| exports.dust = function (setting, config) { | ||
| var settings = (arguments.length > 1) ? setting : {}, | ||
| configs = (arguments.length > 1) ? config : setting, | ||
| renderer; | ||
| var proto = Object.create(oldView.prototype); | ||
|
|
||
| if (!configs || !(configs.specialization || configs.i18n)) { | ||
| return engine.dust(settings); | ||
| } | ||
| // Unfortunately, since we don't know the actual file to resolve until | ||
| // we get request context (in `render`), we can't say whether it exists or not. | ||
| // | ||
| // Express checks that this is truthy to see if it should return an error or | ||
| // run the render, so we hard code it to true. | ||
| proto.path = true; | ||
|
|
||
| proto.lookup = function lookup(name, options, cb) { | ||
| if (arguments.length === 1) { | ||
| // This is the unoverriden constructor calling us. Ignore the call. | ||
| return true; | ||
| } | ||
|
|
||
| var ext = path.extname(name); | ||
|
|
||
| if (conf[ext] && conf[ext].specialization) { | ||
| var nameNoExt = name.slice(0, -ext.length); | ||
| var newName = conf[ext].specialization.resolve(nameNoExt, options) + ext; | ||
| debug("specialization mapped '%s' to '%s'", name, newName); | ||
| name = newName; | ||
| } | ||
|
|
||
| var search = []; | ||
| search.push([].concat(conf[ext] && conf[ext].root ? conf[ext].root : this.root)); | ||
|
|
||
| if (conf[ext] && conf[ext].i18n) { | ||
| var i18n = conf[ext].i18n; | ||
| var locales = []; | ||
| if (options.locale) { | ||
| locales.push(i18n.formatPath(typeof options.locale === 'object' ? options.locale : bcp47.parse(options.locale.replace(/_/g, '-')))); | ||
| } | ||
| if (i18n.fallback) { | ||
| locales.push(i18n.formatPath(i18n.fallback)); | ||
| } | ||
| debug("trying locales %j", locales); | ||
| search.push(locales); | ||
| } | ||
|
|
||
| search.push([name, path.join(path.basename(name), 'index' + ext)]); | ||
|
|
||
| debug('lookup "%s"', name); | ||
|
|
||
| permutron.raw(search, function (candidate, next) { | ||
| var resolved = path.resolve.apply(null, candidate); | ||
| limitStat(resolved, function (err, stat) { | ||
| if (!err && stat.isFile()) { | ||
| debug('found "%s"', resolved); | ||
| cb(null, resolved); | ||
| } else if ((!err && stat.isDirectory()) || (err && err.code === 'ENOENT')) { | ||
| next(); | ||
| } else { | ||
| cb(err); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it looks like there is no need to check that err is defined here..
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I'm not super happy with that logic -- it works, it's correct, but it's not obvious. |
||
| } | ||
| }); | ||
| }, cb); | ||
| }; | ||
|
|
||
| if (configs['view engine'] === 'dust') { | ||
| munger.wrapDustOnLoad('dust', configs, settings.cache); | ||
| /** | ||
| * Render with the given `options` and callback `fn(err, str)`. | ||
| * | ||
| * @param {Object} options | ||
| * @param {Function} fn | ||
| * @api private | ||
| */ | ||
| proto.render = function render(options, fn) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ported from my patch to Express. The term it used was
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does introduce an internal inconsistency to this module. Just thought that was worth noting. I hope to be to a place to provide better feedback than this soon. Just in my first read-through.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. It's not a justification, just an explanation.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding the express patch, I don't see that mentioned in any of the commit messages for this PR. How is the association between this code change and the express patch noted in this software?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh, it's not! expressjs/express#2653 is the pull for express 5
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a link to the README |
||
| aproba('OF', arguments); | ||
| var view = this; | ||
| view.lookupMain(options, function (err) { | ||
| if (err) { | ||
| fn(err); | ||
| } else { | ||
| debug('render "%s"', view.path); | ||
| view.engine(view.path, options, fn); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| /** Resolve the main template for this view | ||
| * | ||
| * @param {function} cb | ||
| * @private | ||
| */ | ||
| proto.lookupMain = function lookupMain(options, cb) { | ||
| if (this.path && this.path !== true) { | ||
| return cb(); | ||
| } | ||
| var view = this; | ||
| var name = path.extname(this.name) === this.ext ? this.name : this.name + this.ext; | ||
| this.lookup(name, options, function (err, path) { | ||
| if (err) { | ||
| return cb(err); | ||
| } else if (!path) { | ||
| var dirs = Array.isArray(view.root) && view.root.length > 1 ? 'directories "' + view.root.slice(0, -1).join('", "') + '" or "' + view.root[view.root.length - 1] + '"' : 'directory "' + view.root + '"'; | ||
| var viewError = new Error('Failed to lookup view "' + name + '" in views ' + dirs); | ||
| viewError.view = view; | ||
| return cb(viewError); | ||
| } else { | ||
| view.path = path; | ||
| cb(); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| function View(name, options) { | ||
| oldView.call(this, name, options); | ||
| } | ||
|
|
||
| // Disabling cache | ||
| // since we add our own caching layer below. (Clone it first so we don't muck with the original object.) | ||
| settings.cache = false; | ||
| View.prototype = proto; | ||
| View.prototype.constructor = View; | ||
| return View; | ||
| } | ||
|
|
||
| module.exports = makeViewClass; | ||
|
|
||
| /** | ||
| * an fs.stat call that limits the number of outstanding requests to 10. | ||
| * | ||
| * @param {String} path | ||
| * @param {Function} cb | ||
| */ | ||
| var pendingStats = []; | ||
| var numPendingStats = 0; | ||
|
|
||
| // For i18n we silently switch to the JS engine for all requests, passing config | ||
| renderer = configs.i18n ? engine.js(settings): engine.dust(settings); | ||
| return munger.wrapEngine(configs, renderer); | ||
| }; | ||
| function limitStat(path, cb) { | ||
| debug('stat "%s"', path); | ||
| if (++numPendingStats > 10) { | ||
| pendingStats.push([path, cb]); | ||
| } else { | ||
| fs.stat(path, dequeue(cb)); | ||
| } | ||
|
|
||
| exports.js = function (setting, config) { | ||
| var settings = (arguments.length > 1) ? setting : {}, | ||
| configs = (arguments.length > 1) ? config : setting, | ||
| renderer; | ||
| function dequeue(cb) { | ||
| return function (err, stat) { | ||
| cb(err, stat); | ||
| var next = pendingStats.shift(); | ||
| if (next) { | ||
| fs.stat(next[0], dequeue(next[1])); | ||
| } else { | ||
| numPendingStats--; | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (!configs || !(configs.specialization || configs.i18n)) { | ||
| return engine.js(settings); | ||
| function normalizeConfigs(config) { | ||
| var out = {}; | ||
| for (var ext in config) { | ||
| if (ext[0] === '.') { | ||
| out[ext] = normalizeConfig(config[ext]); | ||
| } else { | ||
| out['.' + ext] = normalizeConfig(config[ext]); | ||
| } | ||
| } | ||
|
|
||
| if (configs['view engine'] === 'js') { | ||
| munger.wrapDustOnLoad('js', configs, settings.cache); | ||
| return out; | ||
| } | ||
|
|
||
| function normalizeConfig(config) { | ||
| var out = {}; | ||
| if (config.i18n) { | ||
| out.i18n = { | ||
| fallback: config.i18n.fallback && bcp47.parse(config.i18n.fallback.replace(/_/g, '-')), | ||
| formatPath: config.i18n.formatPath || bcp47stringify, | ||
| contentPath: config.i18n.contentPath | ||
| }; | ||
| } | ||
|
|
||
| // Disabling cache | ||
| // since we add our own caching layer below. (Clone it first so we don't muck with the original object.) | ||
| settings.cache = false; | ||
| renderer = engine.js(settings); | ||
| return (configs.specialization) ? munger.wrapEngine(configs, renderer) : renderer; | ||
| }; | ||
| if (config.specialization) { | ||
| out.specialization = karka.create(config.specialization); | ||
| } | ||
|
|
||
| return out; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be
.nyc_output/There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed