Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange behavior for {{#foo}} {{bar}} {{/foo}} when foo is 0 #731

Merged
merged 3 commits into from
Jul 7, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions bench/templates/paths.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
context: { person: { name: "Larry", age: 45 } },
handlebars: "{{person.name}}{{person.age}}{{person.foo}}{{animal.age}}",
dust: "{person.name}{person.age}{person.foo}{animal.age}",
eco: "<%= @person.name %><%= @person.age %><%= @person.foo %><% if @animal: %><%= @animal.age %><% end %>",
mustache: "{{person.name}}{{person.age}}{{person.foo}}{{animal.age}}"
context: { person: { name: {bar: {baz: "Larry"}}, age: 45 } },
handlebars: "{{person.name.bar.baz}}{{person.age}}{{person.foo}}{{animal.age}}",
dust: "{person.name.bar.baz}{person.age}{person.foo}{animal.age}",
eco: "<%= @person.name.bar.baz %><%= @person.age %><%= @person.foo %><% if @animal: %><%= @animal.age %><% end %>",
mustache: "{{person.name.bar.baz}}{{person.age}}{{person.foo}}{{animal.age}}"
};
15 changes: 5 additions & 10 deletions lib/handlebars/compiler/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ Compiler.prototype = {
} else if (this.options.knownHelpersOnly) {
throw new Exception("You specified knownHelpersOnly, but used the unknown helper " + name, sexpr);
} else {
id.falsy = true;

this.ID(id);
this.opcode('invokeHelper', params.length, id.original, sexpr.isRoot);
}
Expand All @@ -298,23 +300,16 @@ Compiler.prototype = {

var name = id.parts[0];
if (!name) {
// Context reference, i.e. `{{foo .}}` or `{{foo ..}}`
this.opcode('pushContext');
} else {
this.opcode('lookupOnContext', id.parts[0]);
}

for(var i=1, l=id.parts.length; i<l; i++) {
this.opcode('lookup', id.parts[i]);
this.opcode('lookupOnContext', id.parts, id.falsy);
}
},

DATA: function(data) {
this.options.data = true;
this.opcode('lookupData', data.id.depth);
var parts = data.id.parts;
for(var i=0, l=parts.length; i<l; i++) {
this.opcode('lookup', parts[i]);
}
this.opcode('lookupData', data.id.depth, data.id.parts);
},

STRING: function(string) {
Expand Down
118 changes: 62 additions & 56 deletions lib/handlebars/compiler/javascript-compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,10 @@ JavaScriptCompiler.prototype = {
// PUBLIC API: You can override these methods in a subclass to provide
// alternative compiled forms for name lookup and buffering semantics
nameLookup: function(parent, name /* , type*/) {
var wrap,
ret;
if (parent.indexOf('depth') === 0) {
wrap = true;
}

if (JavaScriptCompiler.isValidJavaScriptVariableName(name)) {
ret = parent + "." + name;
} else {
ret = parent + "['" + name + "']";
}

if (wrap) {
return '(' + parent + ' && ' + ret + ')';
return parent + "." + name;
} else {
return ret;
return parent + "['" + name + "']";
}
},

Expand Down Expand Up @@ -343,20 +331,7 @@ JavaScriptCompiler.prototype = {
//
// Set the value of the `lastContext` compiler value to the depth
getContext: function(depth) {
if(this.lastContext !== depth) {
this.lastContext = depth;
}
},

// [lookupOnContext]
//
// On stack, before: ...
// On stack, after: currentContext[name], ...
//
// Looks up the value of `name` on the current context and pushes
// it onto the stack.
lookupOnContext: function(name) {
this.push(this.nameLookup('depth' + this.lastContext, name, 'context'));
this.lastContext = depth;
},

// [pushContext]
Expand All @@ -369,32 +344,35 @@ JavaScriptCompiler.prototype = {
this.pushStackLiteral('depth' + this.lastContext);
},

// [resolvePossibleLambda]
//
// On stack, before: value, ...
// On stack, after: resolved value, ...
//
// If the `value` is a lambda, replace it on the stack by
// the return value of the lambda
resolvePossibleLambda: function() {
this.aliases.functionType = '"function"';

this.replaceStack(function(current) {
return "typeof " + current + " === functionType ? " + current + ".apply(depth0) : " + current;
});
},

// [lookup]
// [lookupOnContext]
//
// On stack, before: value, ...
// On stack, after: value[name], ...
// On stack, before: ...
// On stack, after: currentContext[name], ...
//
// Replace the value on the stack with the result of looking
// up `name` on `value`
lookup: function(name) {
this.replaceStack(function(current) {
return current + " == null || " + current + " === false ? " + current + " : " + this.nameLookup(current, name, 'context');
});
// Looks up the value of `name` on the current context and pushes
// it onto the stack.
lookupOnContext: function(parts, falsy) {
/*jshint -W083 */
this.pushContext();
if (!parts) {
return;
}

var len = parts.length;
for (var i = 0; i < len; i++) {
this.replaceStack(function(current) {
var lookup = this.nameLookup(current, parts[i], 'context');
// We want to ensure that zero and false are handled properly for the first element
// of non-chained elements, if the context (falsy flag) needs to have the special
// handling for these values.
if (!falsy && !i && len === 1) {
return ' != null ? ' + lookup + ' : ' + current;
} else {
// Otherwise we can use generic falsy handling
return ' && ' + lookup;
}
}, true);
}
},

// [lookupData]
Expand All @@ -403,12 +381,36 @@ JavaScriptCompiler.prototype = {
// On stack, after: data, ...
//
// Push the data lookup operator
lookupData: function(depth) {
lookupData: function(depth, parts) {
/*jshint -W083 */
if (!depth) {
this.pushStackLiteral('data');
} else {
this.pushStackLiteral('this.data(data, ' + depth + ')');
}
if (!parts) {
return;
}

var len = parts.length;
for (var i = 0; i < len; i++) {
this.replaceStack(function(current) {
return ' && ' + this.nameLookup(current, parts[i], 'data');
}, true);
}
},

// [resolvePossibleLambda]
//
// On stack, before: value, ...
// On stack, after: resolved value, ...
//
// If the `value` is a lambda, replace it on the stack by
// the return value of the lambda
resolvePossibleLambda: function() {
this.aliases.lambda = 'this.lambda';

this.push('lambda(' + this.popStack() + ', depth0)');
},

// [pushStringParam]
Expand Down Expand Up @@ -579,7 +581,7 @@ JavaScriptCompiler.prototype = {
var helper = this.setupHelper(0, name, helperCall);

var helperName = this.lastHelper = this.nameLookup('helpers', name, 'helper');
var nonHelper = this.nameLookup('depth' + this.lastContext, name, 'context');
var nonHelper = '(depth' + this.lastContext + ' && ' + this.nameLookup('depth' + this.lastContext, name, 'context') + ')';

this.push(
'((helper = ' + helperName + ' || ' + nonHelper
Expand Down Expand Up @@ -739,7 +741,7 @@ JavaScriptCompiler.prototype = {
return stack;
},

replaceStack: function(callback) {
replaceStack: function(callback, chain) {
var prefix = '',
inline = this.isInline(),
stack,
Expand All @@ -755,12 +757,16 @@ JavaScriptCompiler.prototype = {
// Literals do not need to be inlined
stack = top.value;
usedLiteral = true;

if (chain) {
prefix = stack;
}
} else {
// Get or create the current stack name for use by the inline
createdStack = !this.stackSlot;
var name = !createdStack ? this.topStackName() : this.incrStack();

prefix = '(' + this.push(name) + ' = ' + top + '),';
prefix = '(' + this.push(name) + ' = ' + top + (chain ? ')' : '),');
stack = this.topStack();
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions lib/handlebars/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ export function template(templateSpec, env) {

// Just add water
var container = {
lambda: function(current, context) {
return typeof current === 'function' ? current.call(context) : current;
},

escapeExpression: Utils.escapeExpression,
invokePartial: invokePartialWrapper,

Expand Down