Making progress on async

src/render.js

@@ -7,32 +7,106 @@ var render = {
 
     renderTemplates: function(templates, scope) {
         assert(scope, 'unable to evalTemplates: scope undefined');
-        var htmlBlocks = [],
+
-            partial,
+        var html = '';
-            promises = [];
+//        var promiseChain = Promise.resolve(''); // create an empty promise to begin the chain;
-        templates.some((template, index) => {
+//        templates.some((template, index) => {
-            if (scope.get('forloop.skip')) return true;
+//            if (scope.get('forloop.skip')) return true;
-            switch (template.type) {
+//            var promiseLink = Promise.resolve('');
-                case 'tag':
+//            switch (template.type) {
-                    promises.push(this.renderTag(template, scope, this.register)
+//                case 'tag':
-                        .then((partial) => {
+//                    // Add Promises to the chain that need to be resolved sequentially
-                            if (partial === undefined) return true;
+//                    promiseLink = this.renderTag(template, scope, this.register)
-                            return htmlBlocks[index] = partial;
+//                        .then((partial) => {
-                        }));
+//                            if (partial === undefined) return true; // basically a noop (do nothing)
-                    break;
+//                            html += partial;
-                case 'html':
+//                        });
-                    promises.push(Promise.resolve(htmlBlocks[index] = template.value));
+//                    promiseChain = promiseChain.then(promiseLink); // add a link to the chain
-                    break;
+//                    break;
-                case 'output':
+//                case 'html':
-                    var val = this.evalOutput(template, scope);
+//                    promiseLink = Promise.resolve(template.value)
-                    htmlBlocks[index] = val === undefined ? '' : stringify(val);
+//                        .then((partial) => {
-                    promises.push(Promise.resolve(htmlBlocks[index]));
+//                            html += partial;
-            }
+//                        });
-        });
+//                    promiseChain = promiseChain.then(promiseLink); // add a link to the chain
-        return Promise.all(promises)
+//                    break;
-            .then((results) => {
+//                case 'output':
-                return htmlBlocks.join('');
+//                    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) {

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));
+                contexts.push(ctx);
-                scope.pop(ctx);
             });
 
-            // This is some pretty tricksy javascript, at least to me.  Bluebird would have made this a lot easier, but
+            // This is some pretty tricksy javascript, at least to me.
-            //  we are trying to not use anything that can't be done in native node Promises.
+            // 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
-            // 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
+            // It's fundamentally equivalent to the following...
-            var lastPromise = scopes.reduce((promise, scope) => {
+            //  emptyPromise.then(renderTemplates(args0).then(renderTemplates(args1).then(renderTemplates(args2)...
-                return promise.then(function(partial) {
+            var lastPromise = contexts.reduce((promise, context) => {
-                    var breakloop = scope.get('forloop.stop');
+                return promise.then((partial) => {
-                    if (breakloop)
+                    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);

tags/tablerow.js

@@ -30,6 +30,7 @@ module.exports = function(liquid) {
             var collection = Liquid.evalExp(this.collection, scope) || [];
 
             var html = '<table>',
+                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) => {
+            // build array of arguments to pass to sequential promises...
-                row = Math.floor(i / cols) + 1;
+            var contexts = collection.slice(offset, offset + limit);
-                col = (i % cols) + 1;
-                if(col === 1){
-                    if(row !== 1){
-                        html += '</tr>';
-                    }
-                    html += `<tr class="row${row}">`;
-                }
 
-                ctx[this.variable] = item;
+            // 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
-                scope.push(ctx);
+            // It's fundamentally equivalent to the following...
-                html += `<td class="col${col}">`;
+            //  emptyPromise.then(renderTemplates(args0).then(renderTemplates(args1).then(renderTemplates(args2)...
-                // todo: replace with sequential promises, see for.js
+            var lastPromise = contexts.reduce((promise, context, currentIndex) => {
-                html += liquid.renderer.renderTemplates(this.templates, scope);
+                return promise.then((partial) => {
-                html += '</td>';
+                    row = Math.floor(currentIndex / cols) + 1;
-                scope.pop(ctx);
+                    col = (currentIndex % cols) + 1;
-            });
+                    if(col === 1) {
-            if(row > 0) html += '</tr>';
+                        if(row !== 1){
-            html += '</table>';
+                            html += '</tr>';
-            return html;
+                        }
+                        html += `<tr class="row${row}">`;
+                    }
+
+                    ctx[this.variable] = context;
+                    scope.push(ctx);
+                    html += `<td class="col${col}">`;
+                    return liquid.renderer.renderTemplates(this.templates, scope)
+                })
+                .then((partial) => {
+                    html += partial;
+                    html += '</td>';
+                    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 += '</tr>';
+                    html += '</table>';
+                    return html;
+                })
+                .catch((error) => {
+                    throw new Error(error);
+                });
         }
     });
 };

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 = '<table>' +
             '<tr class="row1"><td class="col1">a</td><td class="col2">b</td></tr>' +
@@ -261,8 +263,8 @@ describe('tags', function() {
         test(src, dst);
     });
 
-    it('should support include', function() {
+    it.only('should support include', function() {
-        expect(liquid.renderFile('/current.html', ctx)).to.equal('barFOObar');
+        return liquid.renderFile('/current.html', ctx).should.eventually.equal('barFOObar');
     });
 
     it('should support include with relative path', function() {