From 6d0348da5d46fa03a63ee651462fd61764eac479 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Tue, 28 Jan 2014 22:53:31 -0800 Subject: [PATCH] Improve {{> foo}} lookup order We now look for a helper named 'foo' (which can return a dynamically chosen template object) before looking for a global template named 'foo'. This is consistent with the principle that adding a helper should generally override other things with the same name (ie data properties) This change was motivated by frontpage, which had both a template and a helper named 'body'. --- packages/spacebars-compiler/compile_tests.js | 16 ++++----- .../spacebars-compiler/spacebars-compiler.js | 33 ++++++++++--------- packages/spacebars-tests/template_tests.html | 13 ++++++++ packages/spacebars-tests/template_tests.js | 13 ++++++++ packages/ui/fields.js | 12 ++++++- 5 files changed, 62 insertions(+), 25 deletions(-) diff --git a/packages/spacebars-compiler/compile_tests.js b/packages/spacebars-compiler/compile_tests.js index 5b5a04c807..baf30bc683 100644 --- a/packages/spacebars-compiler/compile_tests.js +++ b/packages/spacebars-compiler/compile_tests.js @@ -106,7 +106,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { __content: UI.block(function() { var self = this; return "abc"; @@ -137,7 +137,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { data: self.lookup("bar") }); }; @@ -147,7 +147,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { x: self.lookup("bar") }); }; @@ -157,7 +157,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { data: function() { return Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")); } @@ -169,7 +169,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { x: function() { return Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")); } @@ -184,7 +184,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { __content: UI.block(function() { var self = this; return "aaa"; @@ -200,7 +200,7 @@ Tinytest.add("spacebars - compiler output", function (test) { function() { var self = this; return function() { - return Spacebars.include(Template.foo || self.lookup("foo"), { + return Spacebars.include(self.lookup("foo", {template: true}), { __content: UI.block(function() { var self = this; return "aaa"; @@ -274,4 +274,4 @@ Tinytest.add("spacebars - compiler errors", function (test) { "Unexpected HTML close tag. should have no close tag."); assertStartsWith(getError("{{#each foo}}{{/foo}}"), "Unexpected HTML close tag. should have no close tag."); -}); \ No newline at end of file +}); diff --git a/packages/spacebars-compiler/spacebars-compiler.js b/packages/spacebars-compiler/spacebars-compiler.js index f9a6555bea..89b625c781 100644 --- a/packages/spacebars-compiler/spacebars-compiler.js +++ b/packages/spacebars-compiler/spacebars-compiler.js @@ -170,21 +170,12 @@ var codeGenTemplateTag = function (tag) { codeGenMustache(tag) + '); }'); } else if (tag.type === 'INCLUSION' || tag.type === 'BLOCKOPEN') { var path = tag.path; - var compCode = codeGenPath(path); + var compCode; - if (path.length === 1) { - var compName = path[0]; - if (builtInComponents.hasOwnProperty(compName)) { - compCode = builtInComponents[compName]; - } else { - // toObjectLiteralKey returns `"foo"` or `foo` depending on - // whether `foo` is a safe JavaScript identifier. - var member = toObjectLiteralKey(path[0]); - var templateDotFoo = (member.charAt(0) === '"' ? - 'Template[' + member + ']' : - 'Template.' + member); - compCode = ('(' + templateDotFoo + ' || ' + compCode + ')'); - } + if (path.length === 1 && builtInComponents.hasOwnProperty(path[0])) { + compCode = builtInComponents[path[0]]; + } else { + compCode = codeGenPath(path, {lookupTemplate: true}); } var includeArgs = codeGenInclusionArgs(tag); @@ -305,7 +296,14 @@ var codeGenMustache = function (tag, mustacheType) { // (i.e. it may invalidate the current computation). // // No code is generated to call the result if it's a function. -var codeGenPath = function (path) { +// +// Options: +// +// - lookupTemplate {Boolean} If true, generated code also looks in +// the list of templates. (After helpers, before data context). +// Used when generating code for `{{> foo}}` or `{{#foo}}`. Only +// used for non-dotted paths. +var codeGenPath = function (path, opts) { // Let {{#if content}} check whether this template was invoked via // inclusion or as a block helper. if (builtInComponents.hasOwnProperty(path[0])) { @@ -314,7 +312,10 @@ var codeGenPath = function (path) { return builtInComponents[path[0]]; } - var code = 'self.lookup(' + toJSLiteral(path[0]) + ')'; + var args = [toJSLiteral(path[0])]; + if (opts && opts.lookupTemplate && path.length === 1) + args.push('{template: true}'); + var code = 'self.lookup(' + args.join(', ') + ')'; if (path.length > 1) { code = 'Spacebars.dot(' + code + ', ' + diff --git a/packages/spacebars-tests/template_tests.html b/packages/spacebars-tests/template_tests.html index a5c4da1552..801ad6cef2 100644 --- a/packages/spacebars-tests/template_tests.html +++ b/packages/spacebars-tests/template_tests.html @@ -448,3 +448,16 @@ Hi there! + + + + + + + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index d74ec312a2..078b1f6735 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1340,3 +1340,16 @@ Tinytest.add("spacebars - templates - double", function (test) { run(null, ''); run(undefined, ''); }); + +Tinytest.add("spacebars - templates - inclusion lookup order", function (test) { + // test that {{> foo}} looks for a helper named 'foo' before a + // template named 'foo' + var tmpl = Template.spacebars_template_test_inclusion_lookup; + tmpl.spacebars_template_test_inclusion_lookup_subtmpl = + Template.spacebars_template_test_inclusion_lookup_subtmpl2; + + var div = renderToDiv(tmpl); + test.equal( + stripComments(div.innerHTML), + "This is generated by a helper with the same name."); +}); diff --git a/packages/ui/fields.js b/packages/ui/fields.js index 3bcf1b81d3..4146c5fe8a 100644 --- a/packages/ui/fields.js +++ b/packages/ui/fields.js @@ -29,8 +29,13 @@ var builtInComponents = { }; _extend(UI.Component, { - lookup: function (id) { + // Options: + // + // - template {Boolean} If true, look at the list of templates after + // helpers and before data context. + lookup: function (id, opts) { var self = this; + var template = opts && opts.template; var result; var comp; @@ -58,6 +63,7 @@ _extend(UI.Component, { } else if (_.has(builtInComponents, id)) { return builtInComponents[id]; + // Code to search the global namespace for capitalized names // like component classes, `Template`, `StringUtils.foo`, // etc. @@ -79,6 +85,10 @@ _extend(UI.Component, { // for this? We should definitely not put it on the Handlebars // namespace. result = Handlebars._globalHelpers[id]; + + } else if (template && _.has(Template, id)) { + return Template[id]; + } else { // Resolve id `foo` as `data.foo` (with a "soft dot"). return function (/*arguments*/) {