From 258f4a20737ef5b77ba8b3c08d32003cd1ae2a5b Mon Sep 17 00:00:00 2001 From: harttle Date: Sun, 6 Nov 2016 14:57:48 +0800 Subject: [PATCH] fix include scope.blocks bleeding --- demo/express/app.js | 2 +- demo/express/package.json | 1 + package.json | 2 +- src/render.js | 1 + src/scope.js | 10 ++++-- src/util/underscore.js | 8 +++++ tags/include.js | 7 +++- tags/layout.js | 41 ++++++++++----------- test/tags/layout.js | 75 ++++++++++++++++++++++++++------------- 9 files changed, 95 insertions(+), 52 deletions(-) diff --git a/demo/express/app.js b/demo/express/app.js index 46c15f96f..ffb7bff02 100644 --- a/demo/express/app.js +++ b/demo/express/app.js @@ -8,7 +8,7 @@ var engine = Liquid({ }); app.engine('liquid', engine.express()); // register liquid engine -app.set('views', __dirname); // specify the views directory +app.set('views', ['./partials', './views']); // specify the views directory app.set('view engine', 'liquid'); // set to default app.get('/', function (req, res) { diff --git a/demo/express/package.json b/demo/express/package.json index 9dd830060..b48d0b10c 100644 --- a/demo/express/package.json +++ b/demo/express/package.json @@ -4,6 +4,7 @@ "description": "Express Demo Using Shopify-Liquid", "main": "index.js", "scripts": { + "start": "node index.js", "test": "echo \"Error: no test specified\" && exit 1" }, "author": "harttle", diff --git a/package.json b/package.json index 34dda96c8..b8a4dce6c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "shopify-liquid", - "version": "1.3.0", + "version": "1.3.1", "description": "Liquid template engine for JavaScript, Node.js and Browser", "main": "index.js", "scripts": { diff --git a/src/render.js b/src/render.js index 5687a562e..1bb9a6df2 100644 --- a/src/render.js +++ b/src/render.js @@ -3,6 +3,7 @@ const Promise = require('any-promise'); const mapSeries = require('./util/promise.js').mapSeries; const RenderBreak = require('./util/error.js').RenderBreak; const assert = require('./util/assert.js'); +const _ = require('./util/underscore.js'); var render = { diff --git a/src/scope.js b/src/scope.js index f605915eb..49762009b 100644 --- a/src/scope.js +++ b/src/scope.js @@ -147,13 +147,17 @@ function matchRightBracket(str, begin) { return -1; } -exports.factory = function(_ctx, opts) { +exports.factory = function(ctx, opts) { opts = _.assign({ - strict: false + strict: false, + blocks: {} }, opts); + ctx = _.assign(ctx, { + liquid: opts + }); var scope = Object.create(Scope); scope.opts = opts; - scope.scopes = [_ctx || {}]; + scope.scopes = [ctx]; return scope; }; diff --git a/src/util/underscore.js b/src/util/underscore.js index d45a93fe2..1e8b8cb25 100644 --- a/src/util/underscore.js +++ b/src/util/underscore.js @@ -37,7 +37,15 @@ function isArray(value) { return value instanceof Array; } +function echo(prefix){ + return v => { + console.log('[' + prefix + ']', v); + return v; + }; +} + exports.isString = isString; exports.isArray = isArray; exports.forOwn = forOwn; exports.assign = assign; +exports.echo = echo; diff --git a/tags/include.js b/tags/include.js index b334c435e..4c6d89301 100644 --- a/tags/include.js +++ b/tags/include.js @@ -17,8 +17,12 @@ module.exports = function(liquid) { } }, render: function(scope, hash, register) { - console.log('include', register.root); var filepath = Liquid.evalValue(this.value, scope); + + var reg = scope.get('liquid'); + var originBlocks = reg.blocks; + reg.blocks = {}; + if(this.with){ hash[filepath] = Liquid.evalValue(this.with, scope); } @@ -29,6 +33,7 @@ module.exports = function(liquid) { }) .then((html) => { scope.pop(); + reg.blocks = originBlocks; return html; }); } diff --git a/tags/layout.js b/tags/layout.js index 91333763e..7db0c6a7b 100644 --- a/tags/layout.js +++ b/tags/layout.js @@ -13,24 +13,20 @@ module.exports = function(liquid) { this.layout = match[0]; this.tpls = liquid.parser.parse(remainTokens); }, - render: function(scope) { + render: function(scope, hash, register) { var layout = Liquid.evalValue(this.layout, scope); + var reg = scope.get('liquid'); - var html = ''; - scope.push({}); - // not sure if this first one is needed, since the results are ignored + // render the remaining tokens immediately return liquid.renderer.renderTemplates(this.tpls, scope) - .then((partial) => { - html += partial; - return liquid.getTemplate(layout); - }) - .then((templates) => { - return liquid.renderer.renderTemplates(templates, scope); - }) - .then((partial) => { - scope.pop(); - return partial; - }); + // now register.blocks contains rendered blocks + .then(() => liquid.getTemplate(layout, register.root)) + // push the hash + .then(templates => (scope.push(hash), templates)) + // render the parent + .then(templates => liquid.renderer.renderTemplates(templates, scope)) + // pop the hash + .then(partial => (scope.pop(), partial)); } }); @@ -49,20 +45,21 @@ module.exports = function(liquid) { stream.start(); }, render: function(scope){ - var html = scope.get(`_liquid.blocks.${this.block}`); - var promise = Promise.resolve(''); + var register = scope.get('liquid'); + var html = register.blocks[this.block]; + // if not defined yet if (html === undefined) { - promise = liquid.renderer.renderTemplates(this.tpls, scope) + return liquid.renderer.renderTemplates(this.tpls, scope) .then((partial) => { - scope.set(`_liquid.blocks.${this.block}`, partial); + register.blocks[this.block] = partial; return partial; }); } + // if already defined by desendents else { - scope.set(`_liquid.blocks.${this.block}`, html); - promise = Promise.resolve(html); + register.blocks[this.block] = html; + return Promise.resolve(html); } - return promise; } }); diff --git a/test/tags/layout.js b/test/tags/layout.js index a5f228322..fb96538cd 100644 --- a/test/tags/layout.js +++ b/test/tags/layout.js @@ -12,43 +12,70 @@ describe('tags/layout', function() { extname: '.html' }); }); - beforeEach(function() { - mock({ - '/default-layout.html': 'foo{% block %}Default{% endblock %}foo', - '/multi-blocks-layout.html': 'foo{% block "a"%}{% endblock %}{% block b%}{%endblock%}foo', - '/multi-blocks.html': '{% layout "multi-blocks-layout" %}{%block a%}aaa{%endblock%},{%block b%};{%block c%}ccc{%endblock%};{%endblock%}', - }); - }); afterEach(function() { mock.restore(); }); it('should throw when block not closed', function() { - src = '{% layout "default-layout" %}{%block%}bar'; + mock({ + '/parent.html': 'parent', + }); + src = '{% layout "parent" %}{%block%}A'; return expect(liquid.parseAndRender(src)).to .be.rejectedWith(/tag {%block%} not closed/); }); - it('should support layout', function() { - src = '{% layout "default-layout" %}{%block%}bar{%endblock%}'; + it('should handle anonymous block', function() { + mock({ + '/parent.html': 'X{%block%}{%endblock%}Y', + }); + src = '{% layout "parent.html" %}{%block%}A{%endblock%}'; return expect(liquid.parseAndRender(src)).to - .eventually.equal('foobarfoo'); + .eventually.equal('XAY'); }); - it('should support layout: multiple blocks', function() { - src = '{% layout "multi-blocks-layout" %}' + - '{%block a%}bara{%endblock%}' + - '{%block b%}barb{%endblock%}'; + it('should handle named blocks', function() { + mock({ + '/parent.html': 'X{% block "a"%}{% endblock %}Y{% block b%}{%endblock%}Z', + }); + src = '{% layout "parent.html" %}' + + '{%block a%}A{%endblock%}' + + '{%block b%}B{%endblock%}'; return expect(liquid.parseAndRender(src)).to - .eventually.equal('foobarabarbfoo'); + .eventually.equal('XAYBZ'); }); - it('should support layout: nested 1', function() { - src = '{% layout "multi-blocks" %}{% block a%}A{%endblock%}{%block c%}C{%endblock%}'; + it('should support default block content', function() { + mock({ + '/parent.html': 'X{% block "a"%}A{% endblock %}Y{% block b%}B{%endblock%}Z', + }); + src = '{% layout "parent.html" %}{%block a%}a{%endblock%}'; return expect(liquid.parseAndRender(src)).to - .eventually.equal('fooA;C;foo'); + .eventually.equal('XaYBZ'); }); - it('should support layout: nested 2', function() { - src = '{% layout "multi-blocks" %}{%block c%}C{%endblock%}'; - return expect(liquid.parseAndRender(src)).to - .eventually.equal('fooaaa;C;foo'); + it('should handle nested block', function() { + mock({ + '/grand.html': 'X{%block a%}G{%endblock%}Y', + '/parent.html': '{%layout "grand" %}{%block a%}P{%endblock%}', + '/main.html': '{%layout "parent"%}{%block a%}A{%endblock%}' + }) + return expect(liquid.renderFile('/main.html')).to + .eventually.equal('XAY'); + }); + it('should not bleed scope into included layout', function() { + mock({ + '/parent.html': 'X{%block a%}{%endblock%}Y{%block b%}{%endblock%}Z', + '/main.html': '{%layout "parent"%}'+ + '{%block a%}A{%endblock%}' + + '{%block b%}I{%include "included"%}J{%endblock%}', + '/included.html': '{%layout "parent"%}{%block a%}a{%endblock%}' + }) + return expect(liquid.renderFile('main')).to + .eventually.equal('XAYIXaYZJZ'); + }); + it('should support hash list', function() { + mock({ + '/parent.html': '{{color}}{%block%}{%endblock%}', + '/main.html': '{% layout "parent.html" color:"black"%}{%block%}A{%endblock%}' + }); + return expect(liquid.renderFile('/main.html')).to. + eventually.equal('blackA'); }); - });