fix: compileMacro use new Frame when compile-time

mozilla · Jul 13, 2016 · 7c3e087 · 7c3e087
1 parent cde8ad0
commit 7c3e087
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 20 deletions.
diff --git a/src/compiler.js b/src/compiler.js
@@ -312,12 +312,7 @@ var Compiler = Object.extend({
         var name = node.value;
         var v;
 
-        // if current scope has `node.value` in frame.store, that mean there is
-        // an formal parameter variable name as `node.value`. if just lookup
-        // variable, when parameter name can lookup from scope context, the
-        // parameter may replace by scope value.
-        // see https://github.com/mozilla/nunjucks/issues/774
-        if(!frame.has(name) && (v = frame.lookup(name))) {
+        if((v = frame.lookup(name))) {
             this.emit(v);
         }
         else {
@@ -800,7 +795,7 @@ var Compiler = Object.extend({
         this._compileAsyncLoop(node, frame, true);
     },
 
-    _compileMacro: function(node, frame) {
+    _compileMacro: function(node) {
         var args = [];
         var kwargs = null;
         var funcId = 'macro_' + this.tmpid();
@@ -829,7 +824,7 @@ var Compiler = Object.extend({
         // arguments so support setting positional args with keywords
         // args and passing keyword args as positional args
         // (essentially default values). See runtime.js.
-        frame = frame.push();
+        var frame = new Frame();
         this.emitLines(
             'var ' + funcId + ' = runtime.makeMacro(',
             '[' + argNames.join(', ') + '], ',
@@ -860,7 +855,6 @@ var Compiler = Object.extend({
                           'kwargs["' + name + '"] : ');
                 this._compileExpression(pair.value, frame);
                 this.emitLine(');');
-                frame.addKey(name);
             }, this);
         }
 
@@ -871,7 +865,6 @@ var Compiler = Object.extend({
           this.compile(node.body, frame);
         });
 
-        frame = frame.pop();
         this.emitLine('frame = callerFrame;');
         this.emitLine('return new runtime.SafeString(' + bufferId + ');');
         this.emitLine('});');

diff --git a/src/runtime.js b/src/runtime.js
@@ -17,15 +17,6 @@ var Frame = Obj.extend({
         this.isolateWrites = isolateWrites;
     },
 
-    // add key to store, so we can use is some later
-    addKey: function(key) {
-        this.store[key] = true;
-    },
-
-    has: function(key) {
-        return this.store[key] === true;
-    },
-
     set: function(name, val, resolveUp) {
         // Allow variables with dots by automatically creating the
         // nested structure

diff --git a/tests/compiler.js b/tests/compiler.js
@@ -1583,7 +1583,25 @@
                 '' +
                 '{# calling macro2 #}' +
                 '{{macro2("this should be outputted") }}', {}, {}, function(err, res) {
-                expect(res.trim()).to.eql('this should be outputted');
+                    expect(res.trim()).to.eql('this should be outputted');
+            });
+
+            finish(done);
+        });
+
+        it('should get right value when macro include macro', function(done) {
+            render(
+                '{# macro1 and macro2 definition #}' +
+                '{% macro macro1() %} foo' +
+                '{% endmacro %}' +
+                '' +
+                '{% macro macro2(text="default") %}' +
+                '{{macro1()}}' +
+                '{% endmacro %}' +
+                '' +
+                '{# calling macro2 #}' +
+                '{{macro2("this should be outputted") }}', {}, {}, function(err, res) {
+                    expect(res.trim()).to.eql('foo');
             });
 
             finish(done);