From 3cef60acd191c00df5c8cf9f449a9c59acc0e37c Mon Sep 17 00:00:00 2001 From: Eward Song Date: Wed, 13 Jul 2016 08:31:32 +0800 Subject: [PATCH] fix: compileMacro use new Frame when compile-time --- src/compiler.js | 13 +++---------- src/runtime.js | 10 ---------- tests/compiler.js | 20 +++++++++++++++++++- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/compiler.js b/src/compiler.js index c4bc5c8d..8e3fbcec 100644 --- 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 index e4ca76b6..32d721a1 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -11,21 +11,11 @@ var Frame = Obj.extend({ this.variables = {}; this.parent = parent; this.topLevel = false; - this.store = {}; // if this is true, writes (set) should never propagate upwards past // this frame to its parent (though reads may). 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 index 28fbb994..dd6d4157 100644 --- 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);