Skip to content

Commit

Permalink
Merge pull request #574 from thepechinator/master
Browse files Browse the repository at this point in the history
Make each nunjucks environment have its own globals property
  • Loading branch information
carljm committed Nov 6, 2015
2 parents db6ac93 + 2089038 commit e0cab39
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 48 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Changelog
master (unreleased)
-------------------

* Fix issue with different nunjucks environments sharing same globals. Each environment is now independent.
Thanks Paul Pechin. Merge of [#574](https://github.com/mozilla/nunjucks/pull/574).
* Remove deprecation warning when using the `default` filter without specifying a third argument. Merge of
* Add negative steps support for range function. Thanks Nikita Mostovoy. Merge
of [#575](https://github.com/mozilla/nunjucks/pull/575).
* Remove deprecation warning when using the `default` filter without specifying
Expand Down
21 changes: 13 additions & 8 deletions src/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ var Environment = Obj.extend({
}

this.initCache();

this.globals = globals();
this.filters = {};
this.asyncFilters = [];
this.extensions = {};
Expand Down Expand Up @@ -117,15 +119,15 @@ var Environment = Obj.extend({
},

addGlobal: function(name, value) {
globals[name] = value;
this.globals[name] = value;
return this;
},

getGlobal: function(name) {
if(!globals[name]) {
if(!this.globals[name]) {
throw new Error('global not found: ' + name);
}
return globals[name];
return this.globals[name];
},

addFilter: function(name, func, async) {
Expand Down Expand Up @@ -322,7 +324,10 @@ var Environment = Obj.extend({
});

var Context = Obj.extend({
init: function(ctx, blocks) {
init: function(ctx, blocks, env) {
// Has to be tied to an environment so we can tap into its globals.
this.env = env || new Environment();

// Make a duplicate of ctx
this.ctx = {};
for(var k in ctx) {
Expand All @@ -342,8 +347,8 @@ var Context = Obj.extend({
lookup: function(name) {
// This is one of the most called functions, so optimize for
// the typical case where the name isn't in the globals
if(name in globals && !(name in this.ctx)) {
return globals[name];
if(name in this.env.globals && !(name in this.ctx)) {
return this.env.globals[name];
}
else {
return this.ctx[name];
Expand Down Expand Up @@ -461,7 +466,7 @@ Template = Obj.extend({
else throw err;
}

var context = new Context(ctx || {}, _this.blocks);
var context = new Context(ctx || {}, _this.blocks, _this.env);
var frame = parentFrame ? parentFrame.push() : new Frame();
frame.topLevel = true;
var syncResult = null;
Expand Down Expand Up @@ -518,7 +523,7 @@ Template = Obj.extend({
frame.topLevel = true;

// Run the rootRenderFunc to populate the context with exported vars
var context = new Context(ctx || {}, this.blocks);
var context = new Context(ctx || {}, this.blocks, this.env);
this.rootRenderFunc(this.env,
context,
frame,
Expand Down
67 changes: 36 additions & 31 deletions src/globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,41 +34,46 @@ function joiner(sep) {
};
}

var globals = {
range: function(start, stop, step) {
if(!stop) {
stop = start;
start = 0;
step = 1;
}
else if(!step) {
step = 1;
}

var arr = [];
var i;
if (step > 0) {
for (i=start; i<stop; i+=step) {
arr.push(i);
// Making this a function instead so it returns a new object
// each time it's called. That way, if something like an environment
// uses it, they will each have their own copy.
function globals() {
return {
range: function(start, stop, step) {
if(!stop) {
stop = start;
start = 0;
step = 1;
}
} else {
for (i=start; i>stop; i+=step) {
arr.push(i);
else if(!step) {
step = 1;
}
}
return arr;
},

// lipsum: function(n, html, min, max) {
// },
var arr = [];
var i;
if (step > 0) {
for (i=start; i<stop; i+=step) {
arr.push(i);
}
} else {
for (i=start; i>stop; i+=step) {
arr.push(i);
}
}
return arr;
},

// lipsum: function(n, html, min, max) {
// },

cycler: function() {
return cycler(Array.prototype.slice.call(arguments));
},
cycler: function() {
return cycler(Array.prototype.slice.call(arguments));
},

joiner: function(sep) {
return joiner(sep);
}
};
joiner: function(sep) {
return joiner(sep);
}
};
}

module.exports = globals;
24 changes: 19 additions & 5 deletions tests/globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
return 'Hello ' + arg1;
});

equal('{{ hello("World!") }}', 'Hello World!');
equal('{{ hello("World!") }}', 'Hello World!', env);

finish(done);
});
Expand All @@ -93,8 +93,8 @@
return 'Goodbye ' + arg1;
});

equal('{{ hello("World!") }}', 'Hello World!');
equal('{{ goodbye("World!") }}', 'Goodbye World!');
equal('{{ hello("World!") }}', 'Hello World!', env);
equal('{{ goodbye("World!") }}', 'Goodbye World!', env);

finish(done);
});
Expand All @@ -115,7 +115,8 @@
it('should fail on getting non-existent global', function(done) {
var env = new Environment(new Loader(templatesPath));

expect(env.getGlobal).withArgs('hello1').to.throwError();
// Using this format instead of .withArgs since env.getGlobal uses 'this'
expect(function() { env.getGlobal('hello') }).to.throwError();

finish(done);
});
Expand All @@ -127,8 +128,21 @@
return 'Hello ' + this.lookup('user');
});

equal('{{ hello() }}', { user: 'James' }, 'Hello James');
equal('{{ hello() }}', { user: 'James' }, 'Hello James', env);
finish(done);
});

it('should be exclusive to each environment', function(done) {
var env = new Environment(new Loader(templatesPath));
var env2;

env.addGlobal('hello', 'konichiwa');
env2 = new Environment(new Loader(templatesPath));

// Using this format instead of .withArgs since env2.getGlobal uses 'this'
expect(function() { env2.getGlobal('hello') }).to.throwError();

finish(done);
});
});
})();
15 changes: 11 additions & 4 deletions tests/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@
doneHandler = null;
});

function equal(str, ctx, str2) {
function equal(str, ctx, str2, env) {
if(typeof ctx === 'string') {
env = str2;
str2 = ctx;
ctx = null;
}

var res = render(str, ctx, {});
var res = render(str, ctx, {}, env);
expect(res).to.be(str2);
}

Expand All @@ -50,20 +51,26 @@
return str.replace(/\r\n|\r/g, '\n');
}

function render(str, ctx, opts, cb) {
function render(str, ctx, opts, env, cb) {
if(typeof ctx === 'function') {
cb = ctx;
ctx = null;
opts = null;
env = null;
}
else if(typeof opts === 'function') {
cb = opts;
opts = null;
env = null;
}
else if(typeof env === 'function') {
cb = env;
env = null;
}

opts = opts || {};
opts.dev = true;
var e = new Environment(new Loader(templatesPath), opts);
var e = env || new Environment(new Loader(templatesPath), opts);

var name;
if(opts.filters) {
Expand Down

0 comments on commit e0cab39

Please sign in to comment.