Skip to content

Commit

Permalink
Rework lookup null protector logic
Browse files Browse the repository at this point in the history
- Move the lookup null protection out of `nameLookup` and into that contexts that are aware of the needs for falsy vs. not displayed values.
- Optimize lookup for nested path operations

Fixes #731
  • Loading branch information
kpdecker committed Jul 7, 2014
1 parent 4aad72d commit b5a5c76
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 64 deletions.
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
116 changes: 62 additions & 54 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,30 +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.lambda = 'this.lambda';

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

// [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 @@ -401,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 @@ -577,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 @@ -737,7 +741,7 @@ JavaScriptCompiler.prototype = {
return stack;
},

replaceStack: function(callback) {
replaceStack: function(callback, chain) {
var prefix = '',
inline = this.isInline(),
stack,
Expand All @@ -753,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

0 comments on commit b5a5c76

Please sign in to comment.