carlitoplatanito / gulp-nunjucks-render

[Gulp](https://github.com/wearefractal/gulp) plugin to render [Nunjucks](http://mozilla.github.io/nunjucks/) templates
149 stars 33 forks source link

variables bleed between templates... #6

Closed maranomynet closed 9 years ago

maranomynet commented 10 years ago

I'm using a fairly straightforward gulp task to render all files in a folder.

nunjucksRender.nunjucks.configure(['src/tmpl/']);
return gulp.src('src/tmpl/*.htm')
    .pipe( plumber() )
    .pipe( nunjucksRender() )
    .pipe( gulp.dest( 'site/' ) );

But what I'm seeing is that variables (and macros) defined/set in one template are accessible in another...

If my src/tmpl/ folder contains two files:

src/tmpl/
        page1.htm
        page2.htm

and src/tmpl/page1.htm contains:

{% set customAttr = "class='foobar'" %}
<p {{customAttr}}>Hello World</p>

and src/tmpl/page2.htm contains:

<p {{customAttr}}>Bonjour!</p>

then the rendered output for site/page2.html becomes this:

<p class='foobar'>Bonjour!</p>

which makes life very difficult when one has lots of templates.

silvenon commented 10 years ago

I'm experiencing this too.

carlitoplatanito commented 10 years ago

I'm working through the same issue and the only way I have been able to get around it is by redefining the variables on each page, which is annoying and definitely not the way it should be done. But basically I just do {% set x, y, z = '' %} at the top of each template and then redefine them if needed.

I'm looking for a more elegant solution or seeing if I could possibly flush the variables on each page render but I think part of the problem is node's piping. I will keep you posted as I work though this.

maranomynet commented 10 years ago

Redefining/Resetting every variable on every page is not an acceptable solution - and simply not workable when you have many templates and a slightly non-trivial logic going on.

One possible hack might be to use gulp-foreach to split the file-list into separate streams and feed each individually through gulp-nunjucks-render.

nenadjelovac commented 10 years ago

Damn.. This is the only blocker (that I know of) for me to switch from swig to nunjucks. Hopefully you can find a way to resolve this :)

nenadjelovac commented 9 years ago

@carlosl hey man. Any progress on this one?

maranomynet commented 9 years ago

?

carlitoplatanito commented 9 years ago

Hey, I've been looking at this a little bit for the last hour or so and am able to stop the variables from bleeding if I reconfigure nunjucks each time within the plugin. I have this working and passed the couple of tests I've tried, going to push it to a feature branch.

if (file.base) {
    nunjucks.configure(file.base, {watch: false});
}

try {
    options.name = typeof options.name === 'function' && options.name(file) || file.relative;

    file.contents = new Buffer(nunjucks.renderString(file.contents.toString(), options));
...

This also disables nunjucks internal file watcher and caching, which I'm afraid might cause it to be a lot slower on larger projects, but I don't really have anything too large to test with right now.

Let me know if you can think of any other considerable downsides to this. My few tests haven't shown any noticeable slowdown in building the files. You may also have issues if you want to configure multiple paths, so I'm still exploring a better and more flexible solution.

maranomynet commented 9 years ago

My use of this plugin is on a very small scale for static UI tests/mocks - a couple of dozen files at most. (including 2-3 base templates and a macro library file)

silvenon commented 9 years ago

I will test it on my project and let you know if it's noticeably slower.

carlitoplatanito commented 9 years ago

Sorry guys, the quest continues. It seemed to be working, but it must of just been the order it was processing my files in because once I changed that up it wasn't what I had expected. Back to the drawing board with this one.

maranomynet commented 9 years ago

Any news?

nenadjelovac commented 9 years ago

@carlosl any progress on this?

silvenon commented 9 years ago

It shouldn't be such a problem, just reset the variable on each page. @nenadjelovac do you have a situation where this behavior is extremely inconvenient?

maranomynet commented 9 years ago

This is indeed very inconvenient for any project with more than a handful of variables and pages.

Each time you want to add a new variable, you're forced to manually add it to every single page.

Changing a variable's default value also requires going through every page and selectively update the variable.

All this manual fiddling begins to defeat the purpose of using a templating library in the first place.

BTW, As much as I love this plugin, I have started looking for some other template rendering solution. :-(

silvenon commented 9 years ago

Yeah, I understand, I wasn't thinking big. Maybe gulp-consolidate + nunjucks? I haven't tried it myself yet, but I think you should be able to achieve a similar workflow with that combo.

nenadjelovac commented 9 years ago

@silvenon I think @maranomynet pretty much summed up all issues.

at0g commented 9 years ago

Could it be this line setting options/context in a closure above the call to each file?

In the case of the OP, perhaps the nunjucks engine does not clone the data context, causing the original context to be updated (by reference) when set is called? As the data context is created in a closure well above each through stream, the values would persist and show on multiple renders.

eg. page1.htm - sets customAttr to class='foobar', which in turn sets options.customAttr

page2.htm - passed options as its context, where options.customAttr is already set to class='foobar'

I was working on a fix which is simply using _.cloneDeep(options) to return a new data context for each file. On this fork, I can no longer replicate the OPs issue.

carlitoplatanito commented 9 years ago

This is fixed with @at0g s updates. Updated NPM should work if you do an update. (> 0.2.0)