klei / dust Goto Github PK

Hi everyone.

Is there a way to create a base context to use dust's makeBase method?

When using klei-dust without express.js and when the root and relativeToFile settings are set, then it cannot resolve relative paths.

Example:
We have two templates, parent.dust and child.dust in the folder templates/.

parent.dust:

<div>
  {>child}
</div>

child.dust:

content

And then we try to render the templates:

index.js:

var kleiDust = require('klei-dust');
kleiDust.setOptions({root: 'templates/'});
kleiDust.dust('parent', {}, function (err, out) {
  // It won't come this far, because it cannot find the child template
  console.log(out);
});

Looks like a minor bug. I don't have time to fork/pull-req at the moment. But basically:

this.setHelpers = function (helpers) {
    dust.helper = helpers;
    return dust;
};

That should be dust.helpers.

Side note, I needed to add a filter, but if I use getDust in my express app in order to access the object it initializes it wrong (seems to assume I'm not under express). So I added these two functions to my copy of your code to suit my needs:

this.getFilters = function() {
    return dust.filters;
};  

this.setFilters = function(filters) {
    dust.filters = filters;
}; 

That lets me avoid the getDust() issue. Otherwise I'm loving it. This is the third thing I've tried to get dust to work under current express and it's the first that worked well. Thanks a lot.

Hello - thanks for your project, it was easy to integrate and worked right away. The only problem I had was with the filtering - the @ type statements you can use with the dustjs-linkedin helpers. I'm not sure what the problem was, as for me they began working when I took away your stuff and replaced it with another way to integrate the linkedin version dust with express. I wanted to let you know as this is a great piece of work and perhaps you may like to see if they are working for you (the helper functions).

warm wishes
Stacey

Hi,

Thanks for your support of Dust.

As of Dust 2.5.0, you can set dust.config.whitespace = true to respect whitespace (false is still the default setting for now).

Using this config flag instead of manually clobbering the optimizer avoids a couple bugs like this one.

In the 0.6.2 version keepWhiteSpace has no default. Without setting it the following error is displayed (in this case, through Express):

500 TypeError: Cannot read property 'keepWhiteSpace' of null

If I set:

kleiDust.setOptions({
    'keepWhiteSpace' : false
})

Then the error goes away.

https://github.com/klei-dev/dust/blob/master/lib/klei/dust.js#L123

Since Node's fs package generally reads and writes files asynchronously it isn't hard to run into operating system limits on the number of open files, triggering an EMFILE error. (For examples, search for "emfile node.js".)

I've run into a problem like this when (first) executing a dust template that references a large number of partials.

The Issue

I'd be happy to put together a detailed pull request for this, but for the sake of the initial discussion, allow me to sketch it for your first.

Suppose we have a simple parent template that loops over a list of people:

<!-- (This file is `people.dust`.) -->
<ul>
  {#people}
    <li>{>"partials/person.dust"/}</li>
  {/people}
</ul>

When people contains a small number of objects, this template works as expected, yielding a list of people, each rendered by the partials/person.dust template.

But when people contains a large number of objects the rendering fails with a message like Error: EMFILE, open "partials/person.dust". (In my particular case, "large" is a couple thousand or so.)

Tracing this through klie-dust, it seems the issue revolves around the kleiDust.loadTemplate method (which is invoked by the dust.onLoad method that dust uses when an unrecognized template name is encountered). (In the latest klei-dust version currently distributed via npm) loadTemplate looks like this:

var loadTemplate = function (path, locals, callback) {
  path = self.getFullPath(path, locals);
  var str = getCachedString(path);
  if (str) return callback(null, str);
  fs.readFile(path, 'utf8', function(err, str){
    if (err) return callback(err);
    setCachedString(path, str);
    callback(null, str);
  });
};

Assuming the cache option is set, this method will use a cached version of the file if one is available (via getCachedString). and will add a newly read file to the cache (via setCachedString).

But it so happens that when rendering the people.dust template above, the partials/person.dust template isn't added to the cache until after then entire parent template is processed. That is, if you instrument the code you find that dust.onLoad and in turn, kleiDust.loadTemplate are invoked once for every element of the people array before the callback passed to fs.readFile is ever invoked. E.g., if the partial is invoke twice, you'd seen a sequence like:

dust.onLoad() started
kleiDust.loadTemplate() started
kleiDust.loadTemplate(): Path 'partials/person.dust' not found in string cache, so loading from file.
dust.onLoad() started
kleiDust.loadTemplate() started
kleiDust.loadTemplate(): Path 'partials/person.dust' not found in string cache, so loading from file.

and only later:

kleiDust.setCachedString()
kleiDust.setCachedString()

The partials/person.dust template isn't cached until after all calls to loadTemplate and so it is never found in the cache on that first run. This causes us to load partials/person.dust (via fs.readFile) once for each element of the people array.

One Workaround

One simple but possibly inelegant work-around is to change the asynchronous fs.readFile call into a synchronous fs.readFileSync call. E.g. replacing the fs.readFile block on lines 5-9 above with this:

try {
  str = fs.readFileSync(path, 'utf8');
  setCachedString(path, str);
  callback(null, str);
} catch(err) {
  callback(err);
}

With this change in place, the template is cached by the time dust tries to render the partial a second time, and hence the partials/person.dust file is only loaded once rather than once for every element in the people array. (And subsequent renderings are process exactly like they were before.)

If caching is being used I don't see any glaring issues with loading each template in series rather than in parallel. (If a large number of distinct templates must be loaded there might be a noticeable delay the first time a page is loaded.)

If caching is not being used you could experience delays every time you need to process a large number of distinct templates.

But I've only been looking at klei-dust for an hour or so (having come across it while looking for a solution to a similar problem within consolidate.js), so it is entirely possible that I'm missing something obvious.

Does this fix make any sense to you all? Is there a better way to go about this?

In dustjs, I can supply inline parameter values to partials and it will use the supplied value, but not override the value in the model (if it exists in the model). Below is a code example that demonstrates the issue:

Partial:

Hello {message}

Model:

{ message: 'World' );

Template:

`

Expected Output:

Hello City

Actual Output:

Hello World

Is it possible to stream templates like we can do with dust.stream('template')?

Digging through the code, I see that you have encapsulated everything in such a way that it is physically impossible to add custom helpers anywhere into the system.

If I manually call setHelpers, it is always overridden with {} on the dust calls.

if (self.getOptions().useHelpers) {
            loadHelpers();
        } else
            self.setHelpers({});

Additionally, I cannot access the dust object before the call because getDust() will foolishly initialize the dust object with no parameters... something that always fails.

By the time I can call getDust, the template is already rendered.

My proposed solution is to create an onDustInit method that calls takes a callback with (among other things) the dust object.

I will try to merge this in tomorrow (I have the code done, but do not yet have tests). It works like a charm, and allows custom filters to be added dynamically.

I'd like to submit a pull request to add the following method:

this.getDust = function () {
    if (!isInitialized)
        initializeDust();
    return dust;
};

That way klei-dust can be used with dust.stream(). Any concerns?