MDL-54915 core: JS Blocks must not used shared variables
authorAndrew Nicols <andrew@nicols.co.uk>
Mon, 11 Jul 2016 07:46:49 +0000 (15:46 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Mon, 18 Jul 2016 00:29:52 +0000 (08:29 +0800)
The JS blocks in a template must _not_ use a shared variable as they can be
executed asynchronously and complete in any order.

Instead it is stored in the only local variable available - the current
context.

Since the context can be passed in from elsewhere, we namespace it to
reduce the chance of collission.

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

index 0ffe642..fd310dd 100644 (file)
Binary files a/lib/amd/build/templates.min.js and b/lib/amd/build/templates.min.js differ
index 503c351..d009703 100644 (file)
@@ -45,9 +45,6 @@ define(['core/mustache',
     /** @var {string[]} requiredStrings - Collection of strings found during the rendering of one template */
     var requiredStrings = [];
 
-    /** @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;
 
@@ -182,7 +179,7 @@ define(['core/mustache',
      * @return {string}
      */
     var jsHelper = function(sectionText, helper) {
-        requiredJS.push(helper(sectionText, this));
+        this.jsBlocks.push(helper(sectionText, this));
         return '';
     };
 
@@ -258,7 +255,6 @@ define(['core/mustache',
     var addHelpers = function(context, themeName) {
         currentThemeName = themeName;
         requiredStrings = [];
-        requiredJS = [];
         context.uniqid = uniqid++;
         context.str = function() {
           return stringHelper;
@@ -273,6 +269,7 @@ define(['core/mustache',
           return quoteHelper;
         };
         context.globals = {config: config};
+        context.jsBlocks = [];
         context.currentTheme = themeName;
     };
 
@@ -286,8 +283,8 @@ define(['core/mustache',
      */
     var getJS = function(strings) {
         var js = '';
-        if (requiredJS.length > 0) {
-            js = requiredJS.join(";\n");
+        if (this.jsBlocks.length > 0) {
+            js = this.jsBlocks.join(";\n");
         }
 
         // Re-render to get the final strings.
@@ -403,11 +400,11 @@ define(['core/mustache',
                         // and cause the template to die on the second pass (unbalanced).
 
                         result = treatStringsInContent(result, strings);
-                        deferred.resolve(result, getJS(strings));
+                        deferred.resolve(result, getJS.bind(context)(strings));
                     })
                     .fail(deferred.reject);
                 } else {
-                    deferred.resolve(result.trim(), getJS([]));
+                    deferred.resolve(result.trim(), getJS.bind(context)([]));
                 }
             }
         ).fail(deferred.reject);