Skip to content

Commit

Permalink
MDL-54915 core: JS Blocks must not used shared variables
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
andrewnicols committed Jul 12, 2016
1 parent 47806ea commit 36a9aba
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 10 deletions.
2 changes: 1 addition & 1 deletion lib/amd/build/templates.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 6 additions & 9 deletions lib/amd/src/templates.js
Expand Up @@ -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;

Expand Down Expand Up @@ -181,7 +178,7 @@ define([ 'core/mustache',
* @return {string}
*/
var jsHelper = function(sectionText, helper) {
requiredJS.push(helper(sectionText, this));
this.jsBlocks.push(helper(sectionText, this));
return '';
};

Expand Down Expand Up @@ -257,11 +254,11 @@ define([ 'core/mustache',
var addHelpers = function(context, themeName) {
currentThemeName = themeName;
requiredStrings = [];
requiredJS = [];
context.uniqid = uniqid++;
context.str = function() { return stringHelper; };
context.pix = function() { return pixHelper; };
context.js = function() { return jsHelper; };
context.jsBlocks = [];
context.quote = function() { return quoteHelper; };
context.globals = { config : config };
context.currentTheme = themeName;
Expand All @@ -277,8 +274,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.
Expand Down Expand Up @@ -394,11 +391,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);
Expand Down

0 comments on commit 36a9aba

Please sign in to comment.