Skip to content
This repository was archived by the owner on Jan 25, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
c50c6f9
Directly depend on adaro, and let it bring in dustjs
aredridel May 6, 2015
62624e3
Update built fixtures to use dust 2.7 format
aredridel May 6, 2015
68aae66
Pass specialization via requestOptions
aredridel May 6, 2015
0b18d7d
Fix specialization test and test both paths
aredridel May 7, 2015
400efff
Simplify and make unconditional the onLoad wrappers
aredridel May 7, 2015
1b92a95
Syntactic cleanups
aredridel May 7, 2015
0f440b1
Use dustjs.loadSource and return template instead of source
aredridel May 7, 2015
efa64d0
Rework test suite
aredridel May 8, 2015
71e879a
Check all i18n and specialization combinations in spclAndIntl-js test
aredridel May 8, 2015
e30dfea
Add option `enableMetadata` to allow hooking by in-place content edit…
aredridel Jul 11, 2014
6a8d836
Appease the linter
aredridel Jul 15, 2014
a35d02b
Improve package.json metadata
aredridel May 11, 2015
5e125f7
Rework expressView to mimic the original better and work independentl…
aredridel May 12, 2015
312c099
Switch from tape to tap
aredridel May 19, 2015
055be18
Add nyc for coverage tracking in tests
aredridel May 20, 2015
8b666b5
Replace expressView with one with async lookup
aredridel May 19, 2015
491847b
Make karka unconditional
aredridel May 20, 2015
c34d1d1
Rework to use permutron
aredridel May 21, 2015
fa385dd
Implement i18n lookups cleanly
aredridel May 21, 2015
036e3b0
Remove dead code
aredridel May 30, 2015
c0ec0b6
Update tests to use expressView, rather than the engine directly
aredridel May 30, 2015
eb6b5cb
Make adaro a dev dependency
aredridel May 30, 2015
11dc7f0
Update the README
aredridel Jun 1, 2015
5657129
Remove dead code
aredridel Jun 1, 2015
082ea49
Make error message for not found template slightly more useful
aredridel Jun 1, 2015
d41cfab
Stop checking for i18n on invalid parse check
aredridel Jun 1, 2015
422073f
Check new, better error message
aredridel Jun 1, 2015
5c5cb94
simplify lookupMain and test it
aredridel Jun 1, 2015
f224899
Test having multiple roots to search
aredridel Jun 1, 2015
a03afac
Rearrange what is exported by what
aredridel Jun 1, 2015
985f297
1.0.0-2
aredridel Jun 1, 2015
0d1f0a1
Improve package.json description
aredridel Jun 1, 2015
528b2c1
Add myself as a contributor
aredridel Jun 1, 2015
7c77448
Remove middleware, moved to makara
aredridel Jun 1, 2015
206295a
Remove direct devDependency on istanbul, we have nyc for that
aredridel Jun 1, 2015
7e8422c
Add reference to Express 5 change
aredridel Jun 2, 2015
c600fda
Expose regnerate-test-data script
aredridel Jun 2, 2015
457e310
Improve .gitignore
aredridel Jun 2, 2015
9c8b299
Move error handling into lookup rather than lookupMain to improve erg…
aredridel Jun 2, 2015
5af265a
Bump adaro to get version that always passes the extension to lookup
aredridel Jun 2, 2015
6f99310
1.0.0-3
aredridel Jun 2, 2015
0a7d7c2
Remove useless freshy dependency
aredridel Jun 3, 2015
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@
node_modules/
coverage/
npm-debug.log
*.swp
test/fixtures/tmp/
nyc_output/
Copy link
Member

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/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

120 changes: 60 additions & 60 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,88 +1,88 @@
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.

Lead Maintainer: [Aria Stewart](https://github.com/aredridel)

[![Build Status](https://travis-ci.org/krakenjs/engine-munger.svg?branch=master)](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:
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.

* Dust: Engine types 'js' and 'dust'
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to run npm run-script cover. Is this due to my npm version? (2.5.1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 npm run cover?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. sorry. i was simply using npm cover and then straight to npm run-script cover.. I didn't properly read the doc above and try npm run. That works for me now that I've tried it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome coverage metrics btw!!

```


213 changes: 180 additions & 33 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The 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..

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why fn instead of cb?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ported from my patch to Express. The term it used was fn.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. It's not a justification, just an explanation.

Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
}
Loading