tj / consolidate.js

Template engine consolidation library for node.js
3.48k stars 357 forks source link

[Nunjuck] render method overrides Nunjuck environment pre-customization #311

Closed bpavot closed 6 years ago

bpavot commented 6 years ago

Problem: Using nunjucks custom filters and options get overridden by the nunjucks.render function if Express options.settings.views is set.

This is how I setup the nunjucks filters: cons.requires.nunjucks = nunjucks; var env = cons.requires.nunjucks.configure('templates/views', {watch: false}); env.addFilter('myfilter', function(obj) { return <somecondition>; });

I see this behavior is deprecated already: https://github.com/tj/consolidate.js/blob/887fe28a866aba8ea5bc36ba00e13efd617b9f91/lib/consolidate.js#L1271-L1278

Is there a workaround except overriding the whole render function? Is there a plan to remove the deprecated behavior? I would have guessed the render function should only configure the environment if it was not already.

I can push a pull request if someone can point me to the right direction...

doowb commented 6 years ago

Hi @bpavot if you have an idea on how to make that method better, please submit a PR. I just quickly looked over it and there are things that don't make sense to me. For instance, that piece of code you pointed out won't do anything since there's another set of if blocks that override the env variable using a different nunjucks api.

I'm also not really sure what the deprecated comment means... do older versions of express pass in the options.settings.views property and newer versions don't? or was that an old way for nunjucks to handle configuring an environment, but the new way is different?

It might be helpful to have a simple repository demonstrating the issue that you have and working towards fixing it by using the correct configuration in consolidate.

Thanks for opening the issue.

bpavot commented 6 years ago

I did try to find a decent solution but all involved some major refactoring and/or breaking changes on consolidate.

For now my solution is to not use consolidate but the standard nunjucks way as follows. Feels much more simpler.

var nunjuckEnv = nunjucks.configure('templates/views', {
        autoescape: true,
        express: app,
        watch: false
});

nunjuckEnv.addFilter('myfilter', function(obj) {
    return <somecondition>;
});