Make each nunjucks environment have its own globals property

CHANGELOG.md

@@ -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

src/environment.js

@@ -70,6 +70,8 @@ var Environment = Obj.extend({
         }
 
         this.initCache();
+
+        this.globals = globals();
         this.filters = {};
         this.asyncFilters = [];
         this.extensions = {};
@@ -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) {
@@ -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) {
@@ -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];
@@ -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;
@@ -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,

src/globals.js

@@ -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;

tests/globals.js

@@ -79,7 +79,7 @@
             return 'Hello ' + arg1;
           });
 
-          equal('{{ hello("World!") }}', 'Hello World!');
+          equal('{{ hello("World!") }}', 'Hello World!', env);
 
           finish(done);
         });
@@ -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);
         });
@@ -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);
         });
@@ -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);
+        });
     });
 })();

tests/util.js

@@ -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);
     }
 
@@ -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) {