The "tags" render parameter can now additionally accept a configuration object specifying custom tags and/or a custom escape function for that render case #764
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -529,14 +529,25 @@ Writer.prototype.parse = function parse (template, tags) { | |
| * also be a function that is used to load partial templates on the fly | ||
| * that takes a single argument: the name of the partial. | ||
| * | ||
| * If the optional `config` argument is given here, then it should be an | ||
| * object with a `tags` attribute or an `escape` attribute or both. | ||
| * If an array is passed, then it will be interpreted the same way as | ||
| * a `tags` attribute on a `config` object. | ||
| * | ||
| * The `tags` attribute of a `config` object must be an array with two | ||
| * string values: the opening and closing tags used in the template (e.g. | ||
| * [ "<%", "%>" ]). The default is to mustache.tags. | ||
| * | ||
| * The `escape` attribute of a `config` object must be a function which | ||
| * accepts a string as input and outputs a safely escaped string. | ||
| * If an `escape` function is not provided, then an HTML-safe string | ||
| * escaping function is used as the default. | ||
| */ | ||
| Writer.prototype.render = function render (template, view, partials, config) { | ||
| var tags = this.getConfigTags(config); | ||
| var tokens = this.parse(template, tags); | ||
| var context = (view instanceof Context) ? view : new Context(view, undefined); | ||
| return this.renderTokens(tokens, context, partials, template, config); | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -548,7 +559,7 @@ Writer.prototype.render = function render (template, view, partials, tags) { | |
| * If the template doesn't use higher-order sections, this argument may | ||
| * be omitted. | ||
| */ | ||
| Writer.prototype.renderTokens = function renderTokens (tokens, context, partials, originalTemplate, config) { | ||
| var buffer = ''; | ||
|
|
||
| var token, symbol, value; | ||
|
|
@@ -557,11 +568,11 @@ Writer.prototype.renderTokens = function renderTokens (tokens, context, partials | |
| token = tokens[i]; | ||
| symbol = token[0]; | ||
|
|
||
| if (symbol === '#') value = this.renderSection(token, context, partials, originalTemplate, config); | ||
| else if (symbol === '^') value = this.renderInverted(token, context, partials, originalTemplate, config); | ||
| else if (symbol === '>') value = this.renderPartial(token, context, partials, config); | ||
| else if (symbol === '&') value = this.unescapedValue(token, context); | ||
| else if (symbol === 'name') value = this.escapedValue(token, context, config); | ||
| else if (symbol === 'text') value = this.rawValue(token); | ||
|
|
||
| if (value !== undefined) | ||
|
|
@@ -571,25 +582,25 @@ Writer.prototype.renderTokens = function renderTokens (tokens, context, partials | |
| return buffer; | ||
| }; | ||
|
|
||
| Writer.prototype.renderSection = function renderSection (token, context, partials, originalTemplate, config) { | ||
| var self = this; | ||
| var buffer = ''; | ||
| var value = context.lookup(token[1]); | ||
|
|
||
| // This function is used to render an arbitrary template | ||
| // in the current context by higher-order sections. | ||
| function subRender (template) { | ||
| return self.render(template, context, partials, config); | ||
| } | ||
|
|
||
| if (!value) return; | ||
|
|
||
| if (isArray(value)) { | ||
| for (var j = 0, valueLength = value.length; j < valueLength; ++j) { | ||
| buffer += this.renderTokens(token[4], context.push(value[j]), partials, originalTemplate, config); | ||
| } | ||
| } else if (typeof value === 'object' || typeof value === 'string' || typeof value === 'number') { | ||
| buffer += this.renderTokens(token[4], context.push(value), partials, originalTemplate, config); | ||
| } else if (isFunction(value)) { | ||
| if (typeof originalTemplate !== 'string') | ||
| throw new Error('Cannot use higher-order sections without the original template'); | ||
|
|
@@ -600,18 +611,18 @@ Writer.prototype.renderSection = function renderSection (token, context, partial | |
| if (value != null) | ||
| buffer += value; | ||
| } else { | ||
| buffer += this.renderTokens(token[4], context, partials, originalTemplate, config); | ||
| } | ||
| return buffer; | ||
| }; | ||
|
|
||
| Writer.prototype.renderInverted = function renderInverted (token, context, partials, originalTemplate, config) { | ||
| var value = context.lookup(token[1]); | ||
|
|
||
| // Use JavaScript's definition of falsy. Include empty arrays. | ||
| // See https://github.com/janl/mustache.js/issues/186 | ||
| if (!value || (isArray(value) && value.length === 0)) | ||
| return this.renderTokens(token[4], context, partials, originalTemplate, config); | ||
| }; | ||
|
|
||
| Writer.prototype.indentPartial = function indentPartial (partial, indentation, lineHasNonSpace) { | ||
|
|
@@ -625,8 +636,9 @@ Writer.prototype.indentPartial = function indentPartial (partial, indentation, l | |
| return partialByNl.join('\n'); | ||
| }; | ||
|
|
||
| Writer.prototype.renderPartial = function renderPartial (token, context, partials, config) { | ||
| if (!partials) return; | ||
| var tags = this.getConfigTags(config); | ||
|
|
||
| var value = isFunction(partials) ? partials(token[1]) : partials[token[1]]; | ||
| if (value != null) { | ||
|
|
@@ -637,7 +649,8 @@ Writer.prototype.renderPartial = function renderPartial (token, context, partial | |
| if (tagIndex == 0 && indentation) { | ||
| indentedValue = this.indentPartial(value, indentation, lineHasNonSpace); | ||
| } | ||
| var tokens = this.parse(indentedValue, tags); | ||
| return this.renderTokens(tokens, context, partials, indentedValue, config); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -647,16 +660,38 @@ Writer.prototype.unescapedValue = function unescapedValue (token, context) { | |
| return value; | ||
| }; | ||
|
|
||
| Writer.prototype.escapedValue = function escapedValue (token, context, config) { | ||
| var escape = this.getConfigEscape(config) || mustache.escape; | ||
| var value = context.lookup(token[1]); | ||
| if (value != null) | ||
| return (typeof value === 'number' && escape === mustache.escape) ? String(value) : escape(value); | ||
|
There was a problem hiding this comment. Reading There was a problem hiding this comment. ..I certainly would have expected a test or two to explode badly if the default HTML escaping got removed by this change. Since that's not the case, I guess I'm basically wondering why I'm not really grasping why this introduced change is needed 😄 There was a problem hiding this comment. This ternary condition was from a recent PR (#754) that was intended to optimize performance when writing numeric values. It works fine for escaping HTML, but someone might want their custom escape function to treat numbers differently from the default JS number stringification behavior. (Maybe they want thousands separators. Maybe they want to write hexadecimal values. Etc.) So now template rendering will continue using this optimized path when the escape function is the global default and skip it otherwise. Perhaps one could make an argument for moving this optimization out of this function and into the default escape function? I'm fine making that change, I think it would certainly make the code more clearly legible if nothing else. It would also avoid a potential pitfall with modifying the global escape function and expecting to be able to write numbers differently, like one should now be able to do with an escape function provided via the Though I'd note that this pitfall is preserved behavior and not a newly introduced regression. Changing this behavior may break user code that was expecting their global custom escape function to never have to handle a number value. There was a problem hiding this comment. Since there are other changes in this PR that may break code using custom tags or escape functions that is sensitive to some of these quirks (e.g. if there was any code that relied on the section rendering code discarding the There was a problem hiding this comment. Here is what a more invasive change to the value escaping functionality might look like. All tests continue to pass without changes, so this is essentially backwards compatible, still. But it is likely to change behavior in ways that users relying on global escape function replacement might find inconvenient. If you like it I'll write some tests and make sure everything is solid. If not then the PR as it stands now still satisfies my own needs. There was a problem hiding this comment.
Aaaah very good point indeed, makes perfect sense. I did not think of that aspect when reviewing #754 to be honest. The fact that we've made it so that non-default escape functions won't be able to escape We'll have to reconsider that There was a problem hiding this comment. I fully agree with the rationale behind introducing In danger of sounding too pedantic, I'm wondering if this change could be made in its own PR. Primarily to keep the history of changes as focused and "honest" as possible, without other seemingly unrelated changes introduced. Also feel like what you've just brought up, is severe enough that it deserves the attention a dedicated PR & following discussions would have. There was a problem hiding this comment. I'd like to make sure it's clear that the diff you're referring to will not change behavior when using a global escape function replacement. Whether The abscence or presence of this check There was a problem hiding this comment.
Truly appreciated, impressive piece of work 👏 Thanks for your point of view on what would be a good fit for a followup discussion & PR targeted to what you've raised related to escaping number values. |
||
| }; | ||
|
|
||
| Writer.prototype.rawValue = function rawValue (token) { | ||
| return token[1]; | ||
| }; | ||
|
|
||
| Writer.prototype.getConfigTags = function getConfigTags (config) { | ||
| if (Array.isArray(config)) { | ||
|
There was a problem hiding this comment. Did you consider using our locally defined There was a problem hiding this comment. Very good point. Fixed 👍 |
||
| return config; | ||
| } | ||
| else if (config && typeof config === 'object') { | ||
| return config.tags; | ||
| } | ||
| else { | ||
| return undefined; | ||
| } | ||
| }; | ||
|
|
||
| Writer.prototype.getConfigEscape = function getConfigEscape (config) { | ||
| if (config && typeof config === 'object' && !Array.isArray(config)) { | ||
| return config.escape; | ||
| } | ||
| else { | ||
| return undefined; | ||
| } | ||
| }; | ||
|
|
||
| var mustache = { | ||
| name: 'mustache.js', | ||
| version: '4.0.1', | ||
|
|
@@ -704,19 +739,17 @@ mustache.parse = function parse (template, tags) { | |
| }; | ||
|
|
||
| /** | ||
| * Renders the `template` with the given `view`, `partials`, and `config` | ||
| * using the default writer. | ||
| */ | ||
| mustache.render = function render (template, view, partials, config) { | ||
| if (typeof template !== 'string') { | ||
| throw new TypeError('Invalid template! Template should be a "string" ' + | ||
| 'but "' + typeStr(template) + '" was given as the first ' + | ||
| 'argument for mustache#render(template, view, partials)'); | ||
| } | ||
|
|
||
| return defaultWriter.render(template, view, partials, config); | ||
| }; | ||
|
|
||
| // Export the escaping function so that the user may override it. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bugfix:
tagswas not being passed through before. Now atagsarray or aconfigobject will be passed via therenderfunction argument in a lambda section using thefunction (text, render)API.