MDL-54915 templates: Fix async rendering of js blocks
authorDamyon Wiese <damyon@moodle.com>
Tue, 19 Jul 2016 04:02:28 +0000 (12:02 +0800)
committerDamyon Wiese <damyon@moodle.com>
Tue, 19 Jul 2016 04:55:50 +0000 (12:55 +0800)
Mustache JS helper can overwrite JS blocks when using nested templates.

Each call to render should have it's own scope.

lib/amd/build/templates.min.js
lib/amd/src/templates.js

index 0ffe642..f95b3a4 100644 (file)
Binary files a/lib/amd/build/templates.min.js and b/lib/amd/build/templates.min.js differ
index 503c351..6eeeea5 100644 (file)
@@ -37,22 +37,33 @@ define(['core/mustache',
        ],
        function(mustache, $, ajax, str, notification, coreurl, config, storage, event, Y, Log) {
 
-    // Private variables and functions.
+    // Module variables.
+    /** @var {Number} uniqInstances Count of times this constructor has been called. */
+    var uniqInstances = 0;
 
     /** @var {string[]} templateCache - Cache of already loaded templates */
     var templateCache = {};
 
+    /**
+     * Constructor
+     *
+     * Each call to templates.render gets it's own instance of this class.
+     */
+    var Renderer = function() {
+        this.requiredStrings = [];
+        this.requiredJS = [];
+        this.currentThemeName = '';
+    };
+    // Class variables and functions.
+
     /** @var {string[]} requiredStrings - Collection of strings found during the rendering of one template */
-    var requiredStrings = [];
+    Renderer.prototype.requiredStrings = null;
 
     /** @var {string[]} requiredJS - Collection of js blocks found during the rendering of one template */
-    var requiredJS = [];
-
-    /** @var {Number} uniqid Incrementing value that is changed for every call to render */
-    var uniqid = 1;
+    Renderer.prototype.requiredJS = null;
 
     /** @var {String} themeName for the current render */
-    var currentThemeName = '';
+    Renderer.prototype.currentThemeName = '';
 
     /**
      * Load a template from the cache or local storage or ajax request.
@@ -65,13 +76,13 @@ define(['core/mustache',
      * @param {Boolean} async If false - this function will not return until the promises are resolved.
      * @return {Promise} JQuery promise object resolved when the template has been fetched.
      */
-    var getTemplate = function(templateName, async) {
+    Renderer.prototype.getTemplate = function(templateName, async) {
         var deferred = $.Deferred();
         var parts = templateName.split('/');
         var component = parts.shift();
         var name = parts.shift();
 
-        var searchKey = currentThemeName + '/' + templateName;
+        var searchKey = this.currentThemeName + '/' + templateName;
 
         // First try request variables.
         if (searchKey in templateCache) {
@@ -94,7 +105,7 @@ define(['core/mustache',
             args: {
                 component: component,
                 template: name,
-                themename: currentThemeName
+                themename: this.currentThemeName
             }
         }], async, false);
 
@@ -120,10 +131,10 @@ define(['core/mustache',
      * @param {string} name The partial name to load.
      * @return {string}
      */
-    var partialHelper = function(name) {
+    Renderer.prototype.partialHelper = function(name) {
         var template = '';
 
-        getTemplate(name, false).done(
+        this.getTemplate(name, false).done(
             function(source) {
                 template = source;
             }
@@ -137,11 +148,12 @@ define(['core/mustache',
      *
      * @method pixHelper
      * @private
+     * @param {object} context The mustache context
      * @param {string} sectionText The text to parse arguments from.
      * @param {function} helper Used to render the alt attribute of the text.
      * @return {string}
      */
-    var pixHelper = function(sectionText, helper) {
+    Renderer.prototype.pixHelper = function(context, sectionText, helper) {
         var parts = sectionText.split(',');
         var key = '';
         var component = '';
@@ -167,8 +179,8 @@ define(['core/mustache',
             ]
         };
         // We forced loading of this early, so it will be in the cache.
-        var template = templateCache[currentThemeName + '/core/pix_icon'];
-        result = mustache.render(template, templatecontext, partialHelper);
+        var template = templateCache[this.currentThemeName + '/core/pix_icon'];
+        result = mustache.render(template, templatecontext, this.partialHelper.bind(this));
         return result.trim();
     };
 
@@ -177,12 +189,13 @@ define(['core/mustache',
      *
      * @method jsHelper
      * @private
+     * @param {object} context The current mustache context.
      * @param {string} sectionText The text to save as a js block.
      * @param {function} helper Used to render the block.
      * @return {string}
      */
-    var jsHelper = function(sectionText, helper) {
-        requiredJS.push(helper(sectionText, this));
+    Renderer.prototype.jsHelper = function(context, sectionText, helper) {
+        this.requiredJS.push(helper(sectionText, context));
         return '';
     };
 
@@ -192,11 +205,12 @@ define(['core/mustache',
      *
      * @method stringHelper
      * @private
+     * @param {object} context The current mustache context.
      * @param {string} sectionText The text to parse the arguments from.
      * @param {function} helper Used to render subsections of the text.
      * @return {string}
      */
-    var stringHelper = function(sectionText, helper) {
+    Renderer.prototype.stringHelper = function(context, sectionText, helper) {
         var parts = sectionText.split(',');
         var key = '';
         var component = '';
@@ -213,15 +227,15 @@ define(['core/mustache',
 
         if (param !== '') {
             // Allow variable expansion in the param part only.
-            param = helper(param, this);
+            param = helper(param, context);
         }
         // Allow json formatted $a arguments.
         if ((param.indexOf('{') === 0) && (param.indexOf('{{') !== 0)) {
             param = JSON.parse(param);
         }
 
-        var index = requiredStrings.length;
-        requiredStrings.push({key: key, component: component, param: param});
+        var index = this.requiredStrings.length;
+        this.requiredStrings.push({key: key, component: component, param: param});
         return '{{_s' + index + '}}';
     };
 
@@ -230,12 +244,13 @@ define(['core/mustache',
      *
      * @method quoteHelper
      * @private
+     * @param {object} context The current mustache context.
      * @param {string} sectionText The text to parse the arguments from.
      * @param {function} helper Used to render subsections of the text.
      * @return {string}
      */
-    var quoteHelper = function(sectionText, helper) {
-        var content = helper(sectionText.trim(), this);
+    Renderer.prototype.quoteHelper = function(context, sectionText, helper) {
+        var content = helper(sectionText.trim(), context);
 
         // Escape the {{ and the ".
         // This involves wrapping {{, and }} in change delimeter tags.
@@ -255,23 +270,23 @@ define(['core/mustache',
      * @param {Object} context Simple types used as the context for the template.
      * @param {String} themeName We set this multiple times, because there are async calls.
      */
-    var addHelpers = function(context, themeName) {
-        currentThemeName = themeName;
-        requiredStrings = [];
-        requiredJS = [];
-        context.uniqid = uniqid++;
+    Renderer.prototype.addHelpers = function(context, themeName) {
+        this.currentThemeName = themeName;
+        this.requiredStrings = [];
+        this.requiredJS = [];
+        context.uniqid = (uniqInstances++);
         context.str = function() {
-          return stringHelper;
-        };
+          return this.stringHelper.bind(this, context);
+        }.bind(this);
         context.pix = function() {
-          return pixHelper;
-        };
+          return this.pixHelper.bind(this, context);
+        }.bind(this);
         context.js = function() {
-          return jsHelper;
-        };
+          return this.jsHelper.bind(this, context);
+        }.bind(this);
         context.quote = function() {
-          return quoteHelper;
-        };
+          return this.quoteHelper.bind(this, context);
+        }.bind(this);
         context.globals = {config: config};
         context.currentTheme = themeName;
     };
@@ -284,14 +299,14 @@ define(['core/mustache',
      * @param {string[]} strings Replacement strings.
      * @return {string}
      */
-    var getJS = function(strings) {
+    Renderer.prototype.getJS = function(strings) {
         var js = '';
-        if (requiredJS.length > 0) {
-            js = requiredJS.join(";\n");
+        if (this.requiredJS.length > 0) {
+            js = this.requiredJS.join(";\n");
         }
 
         // Re-render to get the final strings.
-        return treatStringsInContent(js, strings);
+        return this.treatStringsInContent(js, strings);
     };
 
     /**
@@ -311,7 +326,7 @@ define(['core/mustache',
      * @param {Array} strings The strings to replace with.
      * @return {String} The treated content.
      */
-    var treatStringsInContent = function(content, strings) {
+    Renderer.prototype.treatStringsInContent = function(content, strings) {
         var pattern = /{{_s\d+}}/,
             treated,
             index,
@@ -373,26 +388,26 @@ define(['core/mustache',
      * @param {String} themeName Name of the current theme.
      * @return {Promise} object
      */
-    var doRender = function(templateSource, context, themeName) {
+    Renderer.prototype.doRender = function(templateSource, context, themeName) {
         var deferred = $.Deferred();
 
-        currentThemeName = themeName;
+        this.currentThemeName = themeName;
 
         // Make sure we fetch this first.
-        var loadPixTemplate = getTemplate('core/pix_icon', true);
+        var loadPixTemplate = this.getTemplate('core/pix_icon', true);
 
         loadPixTemplate.done(
             function() {
-                addHelpers(context, themeName);
+                this.addHelpers(context, themeName);
                 var result = '';
                 try {
-                    result = mustache.render(templateSource, context, partialHelper);
+                    result = mustache.render(templateSource, context, this.partialHelper.bind(this));
                 } catch (ex) {
                     deferred.reject(ex);
                 }
 
-                if (requiredStrings.length > 0) {
-                    str.get_strings(requiredStrings)
+                if (this.requiredStrings.length > 0) {
+                    str.get_strings(this.requiredStrings)
                     .then(function(strings) {
 
                         // Why do we not do another call the render here?
@@ -402,14 +417,14 @@ define(['core/mustache',
                         // would get inserted in the template in the first pass
                         // and cause the template to die on the second pass (unbalanced).
 
-                        result = treatStringsInContent(result, strings);
-                        deferred.resolve(result, getJS(strings));
-                    })
+                        result = this.treatStringsInContent(result, strings);
+                        deferred.resolve(result, this.getJS(strings));
+                    }.bind(this))
                     .fail(deferred.reject);
                 } else {
-                    deferred.resolve(result.trim(), getJS([]));
+                    deferred.resolve(result.trim(), this.getJS([]));
                 }
-            }
+            }.bind(this)
         ).fail(deferred.reject);
         return deferred.promise();
     };
@@ -468,11 +483,58 @@ define(['core/mustache',
         }
     };
 
+    /**
+     * Load a template and call doRender on it.
+     *
+     * @method render
+     * @private
+     * @param {string} templateName - should consist of the component and the name of the template like this:
+     *                              core/menu (lib/templates/menu.mustache) or
+     *                              tool_bananas/yellow (admin/tool/bananas/templates/yellow.mustache)
+     * @param {Object} context - Could be array, string or simple value for the context of the template.
+     * @param {string} themeName - Name of the current theme.
+     * @return {Promise} JQuery promise object resolved when the template has been rendered.
+     */
+    Renderer.prototype.render = function(templateName, context, themeName) {
+        var deferred = $.Deferred();
+
+        if (typeof (themeName) === "undefined") {
+            // System context by default.
+            themeName = config.theme;
+        }
+
+        this.currentThemeName = themeName;
+
+        var loadTemplate = this.getTemplate(templateName, true);
+
+        loadTemplate.done(
+            function(templateSource) {
+                var renderPromise = this.doRender(templateSource, context, themeName);
+
+                renderPromise.done(
+                    function(result, js) {
+                        deferred.resolve(result, js);
+                    }
+                ).fail(
+                    function(ex) {
+                        deferred.reject(ex);
+                    }
+                );
+            }.bind(this)
+        ).fail(
+            function(ex) {
+                deferred.reject(ex);
+            }
+        );
+        return deferred.promise();
+    };
+
 
     return /** @alias module:core/templates */ {
         // Public variables and functions.
         /**
-         * Load a template and call doRender on it.
+         * Every call to render creates a new instance of the class and calls render on it. This
+         * means each render call has it's own class variables.
          *
          * @method render
          * @private
@@ -484,37 +546,8 @@ define(['core/mustache',
          * @return {Promise} JQuery promise object resolved when the template has been rendered.
          */
         render: function(templateName, context, themeName) {
-            var deferred = $.Deferred();
-
-            if (typeof (themeName) === "undefined") {
-                // System context by default.
-                themeName = config.theme;
-            }
-
-            currentThemeName = themeName;
-
-            var loadTemplate = getTemplate(templateName, true);
-
-            loadTemplate.done(
-                function(templateSource) {
-                    var renderPromise = doRender(templateSource, context, themeName);
-
-                    renderPromise.done(
-                        function(result, js) {
-                            deferred.resolve(result, js);
-                        }
-                    ).fail(
-                        function(ex) {
-                            deferred.reject(ex);
-                        }
-                    );
-                }
-            ).fail(
-                function(ex) {
-                    deferred.reject(ex);
-                }
-            );
-            return deferred.promise();
+            var renderer = new Renderer();
+            return renderer.render(templateName, context, themeName);
         },
 
         /**