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

Already on GitHub? Sign in to your account

Support lookup of model values via object.get(key) #97

Merged
merged 2 commits into from Nov 25, 2012

Conversation

Projects
None yet
4 participants
Contributor

MattCheely commented Jul 18, 2012

We've been using hogan with backbone, and I've been thinking for a while now that it would be nice if hogan could find model properties using model.get('foo'), rather than requiring templates to sometimes dig into model.attributes & sometimes calling model.isFoo for methods.

This patch causes templates to fall back to looking up keys in objects with get() if the key can't be found directly. Other options & approaches I considered were:

  • Enable this behavior via an option passed to Hogan.compile (similar to disableLambda)
  • Allow setting a custom name resolution method option in Hogan.compile. (Here's a gun aimed at your foot, have fun!) i.e:
Hogan.compile(template, {
  resolveName: function(name, object) {
    return object[name];   //this would not work, but you get the idea
  }
});
Support lookup of model values via object.get(key)
This is potentially useful for users using hogan.js with backbone
or similar framworks whose model classes access data indirectly
via get()

This pull request passes (merged c019517 into f753c7c).

@sayrer sayrer commented on an outdated diff Jul 18, 2012

lib/template.js
@@ -254,6 +256,24 @@ var Hogan = {};
};
+ //Find a key in an object
+ function findInScope(key, scope) {
+ var val;
+
+ if (scope && typeof scope == 'object') {
+
+ if (scope[key] != null) {
+ val = scope[key];
+
+ // try lookup with get for backbone or similar model data
+ } else if (scope.get && typeof scope.get == 'function' && scope.get(key) != null) {
@sayrer

sayrer Jul 18, 2012

Collaborator

This calls scope.get twice.

This pull request passes (merged f6b0c5b into f753c7c).

Contributor

MattCheely commented Jul 18, 2012

Thanks for catching that. I realized after your comment that there's no reason to check for null in that case anyway, as the caller of findInScope always has to check to see if the output == null to know if it should keep searching other scopes.

Contributor

MattCheely commented Aug 1, 2012

Is there any interest in this feature? If not, I'll close the pull request and we can maintain a local fork with the feature in our private repos.

Collaborator

sayrer commented Sep 21, 2012

Yeah, I'll take the feature. It is a pretty common request. I think the perf hit will be substantial, so I'm going to make it an option.

@fat fat merged commit f6b0c5b into twitter:master Nov 25, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment