diff --git a/src/render.js b/src/render.js index d146cc4a8..9e16e65c1 100644 --- a/src/render.js +++ b/src/render.js @@ -7,32 +7,106 @@ var render = { renderTemplates: function(templates, scope) { assert(scope, 'unable to evalTemplates: scope undefined'); - var htmlBlocks = [], - partial, - promises = []; - templates.some((template, index) => { - if (scope.get('forloop.skip')) return true; - switch (template.type) { - case 'tag': - promises.push(this.renderTag(template, scope, this.register) - .then((partial) => { - if (partial === undefined) return true; - return htmlBlocks[index] = partial; - })); - break; - case 'html': - promises.push(Promise.resolve(htmlBlocks[index] = template.value)); - break; - case 'output': - var val = this.evalOutput(template, scope); - htmlBlocks[index] = val === undefined ? '' : stringify(val); - promises.push(Promise.resolve(htmlBlocks[index])); - } - }); - return Promise.all(promises) - .then((results) => { - return htmlBlocks.join(''); + + var html = ''; +// var promiseChain = Promise.resolve(''); // create an empty promise to begin the chain; +// templates.some((template, index) => { +// if (scope.get('forloop.skip')) return true; +// var promiseLink = Promise.resolve(''); +// switch (template.type) { +// case 'tag': +// // Add Promises to the chain that need to be resolved sequentially +// promiseLink = this.renderTag(template, scope, this.register) +// .then((partial) => { +// if (partial === undefined) return true; // basically a noop (do nothing) +// html += partial; +// }); +// promiseChain = promiseChain.then(promiseLink); // add a link to the chain +// break; +// case 'html': +// promiseLink = Promise.resolve(template.value) +// .then((partial) => { +// html += partial; +// }); +// promiseChain = promiseChain.then(promiseLink); // add a link to the chain +// break; +// case 'output': +// var val = this.evalOutput(template, scope); +// promiseLink = Promise.resolve(val === undefined ? '' : stringify(val)) +// .then((partial) => { +// html += partial; +// }); +// promiseChain = promiseChain.then(promiseLink); // add a link to the chain +// } +// }); +// return promiseChain.then((result) => { +// // this should happen after all of the above promises are finished, and they should have resolved in order +// return html; +// }); + + + // This executes an array of promises sequentially for every template in the templates array - http://webcache.googleusercontent.com/search?q=cache:rNbMUn9TPtkJ:joost.vunderink.net/blog/2014/12/15/processing-an-array-of-promises-sequentially-in-node-js/+&cd=5&hl=en&ct=clnk&gl=us + // It's fundamentally equivalent to the following... + // emptyPromise.then(renderTag(template0).then(renderTag(template1).then(renderTag(template2)... + var lastPromise = templates.reduce((promise, template) => { + return promise.then((partial) => { + if (scope.get('forloop.skip')) { + return Promise.resolve(''); + } + if (scope.get('forloop.stop')) { + throw new Error('forloop.stop'); // this will stop/break the sequential promise chain and go to the catch + } + + var promiseLink = Promise.resolve(''); + switch (template.type) { + case 'tag': + // Add Promises to the chain + promiseLink = this.renderTag(template, scope, this.register) + .then((partial) => { + if (partial === undefined) { + return true; // basically a noop (do nothing) + } + return html += partial; + }); + break; + case 'html': + promiseLink = Promise.resolve(template.value) + .then((partial) => { + return html += partial; + }); + break; + case 'output': + var val = this.evalOutput(template, scope); + promiseLink = Promise.resolve(val === undefined ? '' : stringify(val)) + .then((partial) => { + return html += partial; + }); + break; + } + + return promiseLink; + }) + .catch((error) => { + if (error === 'forloop.stop') { + // the error is a controlled, purposeful stop. so just return the html that we have up to this point + return html; + } else { + // rethrow actual error + throw new Error(error); + } }); + }, Promise.resolve('')); // start the reduce chain with a resolved Promise. After first run, the "promise" argument + // in our reduce callback will be the returned promise from our "then" above. In this + // case, that's the promise returned from this.renderTag or a resolved promise with raw html. + + return lastPromise + .then(() => { + return html; + }) + .catch((error) => { + throw new Error(error); + }); + }, renderTag: function(template, scope, register) { diff --git a/tags/for.js b/tags/for.js index 8c0444377..b69773965 100644 --- a/tags/for.js +++ b/tags/for.js @@ -47,7 +47,10 @@ module.exports = function(liquid) { collection = collection.slice(offset, offset + limit); if(this.reversed) collection.reverse(); - var scopes = []; + // for needs to execute the promises sequentially, not just resolve them sequentially, due to break and continue. + // We can't just loop through executing everything then resolve them all sequentially like we do for render.renderTemplates + // First, we build the array of parameters we are going to use for each call to renderTemplates + var contexts = []; collection.some((item, i) => { ctx[this.variable] = item; ctx.forloop = { @@ -61,26 +64,31 @@ module.exports = function(liquid) { stop: false, skip: false }; - // todo: verify scope management is good here. Make sure we don't consume too many resources here with the clone. - // Is there a simpler solution? - scope.push(ctx); // We are just putting together an array of the arguments we will be passing to our sequential promises - scopes.push(_.clone(scope)); - scope.pop(ctx); + contexts.push(ctx); }); - // This is some pretty tricksy javascript, at least to me. Bluebird would have made this a lot easier, but - // we are trying to not use anything that can't be done in native node Promises. - // This basically just processes an array of promises sequentially for every argument in the array - http://webcache.googleusercontent.com/search?q=cache:rNbMUn9TPtkJ:joost.vunderink.net/blog/2014/12/15/processing-an-array-of-promises-sequentially-in-node-js/+&cd=5&hl=en&ct=clnk&gl=us - var lastPromise = scopes.reduce((promise, scope) => { - return promise.then(function(partial) { - var breakloop = scope.get('forloop.stop'); - if (breakloop) + // This is some pretty tricksy javascript, at least to me. + // This executes an array of promises sequentially for every argument in the contexts array - http://webcache.googleusercontent.com/search?q=cache:rNbMUn9TPtkJ:joost.vunderink.net/blog/2014/12/15/processing-an-array-of-promises-sequentially-in-node-js/+&cd=5&hl=en&ct=clnk&gl=us + // It's fundamentally equivalent to the following... + // emptyPromise.then(renderTemplates(args0).then(renderTemplates(args1).then(renderTemplates(args2)... + var lastPromise = contexts.reduce((promise, context) => { + return promise.then((partial) => { + if (scope.get('forloop.stop')) { throw new Error('forloop.stop'); // this will stop the sequential promise chain + } html += partial; + // todo: Make sure our scope management is sound here. Create some tests that revolve around loops + // with sections that take differing amounts of time to complete. Make sure the order is maintained + // and scope doesn't bleed over into other renderTemplate calls. + scope.push(context); return liquid.renderer.renderTemplates(this.templates, scope); }) + .then((partial) => { + scope.pop(context); + return partial; + }) .catch((error) => { if (error === 'forloop.stop') { // the error is a controlled, purposeful stop. so just return the html that we have up to this point @@ -94,9 +102,9 @@ module.exports = function(liquid) { // in our reduce callback will be the returned promise from our "then" above. In this // case, the promise returned from liquid.renderer.renderTemplates. - lastPromise + return lastPromise .then(() => { - return Promise.resolve(html); + return html; }) .catch((error) => { throw new Error(error); diff --git a/tags/tablerow.js b/tags/tablerow.js index 1640c5a65..ecf24cdaf 100644 --- a/tags/tablerow.js +++ b/tags/tablerow.js @@ -30,6 +30,7 @@ module.exports = function(liquid) { var collection = Liquid.evalExp(this.collection, scope) || []; var html = '', + promiseChain = Promise.resolve(''); // create an empty promise to begin the chain ctx = {}, length = collection.length; var offset = hash.offset || 0; @@ -38,27 +39,50 @@ module.exports = function(liquid) { var cols = hash.cols, row, col; if (!cols) throw new Error(`illegal cols: ${cols}`); - collection.slice(offset, offset + limit).some((item, i) => { - row = Math.floor(i / cols) + 1; - col = (i % cols) + 1; - if(col === 1){ - if(row !== 1){ - html += ''; - } - html += ``; - } + // build array of arguments to pass to sequential promises... + var contexts = collection.slice(offset, offset + limit); - ctx[this.variable] = item; - scope.push(ctx); - html += `'; - scope.pop(ctx); - }); - if(row > 0) html += ''; - html += '
`; - // todo: replace with sequential promises, see for.js - html += liquid.renderer.renderTemplates(this.templates, scope); - html += '
'; - return html; + // This executes an array of promises sequentially for every argument in the contexts array - http://webcache.googleusercontent.com/search?q=cache:rNbMUn9TPtkJ:joost.vunderink.net/blog/2014/12/15/processing-an-array-of-promises-sequentially-in-node-js/+&cd=5&hl=en&ct=clnk&gl=us + // It's fundamentally equivalent to the following... + // emptyPromise.then(renderTemplates(args0).then(renderTemplates(args1).then(renderTemplates(args2)... + var lastPromise = contexts.reduce((promise, context, currentIndex) => { + return promise.then((partial) => { + row = Math.floor(currentIndex / cols) + 1; + col = (currentIndex % cols) + 1; + if(col === 1) { + if(row !== 1){ + html += ''; + } + html += ``; + } + + ctx[this.variable] = context; + scope.push(ctx); + html += ``; + return liquid.renderer.renderTemplates(this.templates, scope) + }) + .then((partial) => { + html += partial; + html += ''; + scope.pop(context); + return partial; // I think this is currently unused (partial is not used in the above "then") + }) + .catch((error) => { + throw new Error(error); + }); + }, Promise.resolve()); // start the reduce chain with a resolved Promise. After first run, the "promise" argument + // in our reduce callback will be the returned promise from our "then" above. In this + // case, the promise returned from liquid.renderer.renderTemplates. + + lastPromise + .then(() => { + if(row > 0) html += ''; + html += ''; + return html; + }) + .catch((error) => { + throw new Error(error); + }); } }); }; diff --git a/test/tags.js b/test/tags.js index 063e4443b..35df7ce9e 100644 --- a/test/tags.js +++ b/test/tags.js @@ -1,3 +1,5 @@ +// temporary +var Promise = require('any-promise'); const chai = require("chai"); const should = chai.should(); const expect = chai.expect; @@ -207,7 +209,7 @@ describe('tags', function() { test('{% decrement one %}{{one}}', '0'); }); - it.only('should support tablerow', function() { + it('should support tablerow', function() { src = '{% tablerow i in alpha cols:2 %}{{ i }}{% endtablerow %}'; dst = '' + '' + @@ -261,8 +263,8 @@ describe('tags', function() { test(src, dst); }); - it('should support include', function() { - expect(liquid.renderFile('/current.html', ctx)).to.equal('barFOObar'); + it.only('should support include', function() { + return liquid.renderFile('/current.html', ctx).should.eventually.equal('barFOObar'); }); it('should support include with relative path', function() {
ab