From 135aefbdee388b8d14a8cb8c3056501d66cd85db Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Sun, 2 Feb 2014 12:26:48 -0800 Subject: [PATCH] Fix broken newargs semantics All tests pass (except one which is a todo on shark). If `{{> foo bar}}` is to desugar into `{{#with bar}}{{> foo}}{{/with}}`, there needs to be an explicit block in the generated code in which `self.lookup("foo")` binds the correct data context (`bar`). So we now generate a UI.With rather than trying to call it from Spacebars.include. This simplifies Spacebars.include, which is nice. Define `lookupTemplate` because `self.lookup(.., {template: true})` pretty-prints bigly. --- packages/spacebars-compiler/compile_tests.js | 116 +++++++++--------- .../spacebars-compiler/spacebars-compiler.js | 71 ++++++----- packages/spacebars-tests/template_tests.js | 4 +- packages/spacebars/spacebars-runtime.js | 72 +++++------ packages/ui/fields.js | 3 + 5 files changed, 129 insertions(+), 137 deletions(-) diff --git a/packages/spacebars-compiler/compile_tests.js b/packages/spacebars-compiler/compile_tests.js index d8858a1f99..35e51a836c 100644 --- a/packages/spacebars-compiler/compile_tests.js +++ b/packages/spacebars-compiler/compile_tests.js @@ -105,13 +105,10 @@ Tinytest.add("spacebars - compiler output", function (test) { run("{{#foo}}abc{{/foo}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), null, { - content: UI.block(function() { - var self = this; - return "abc"; - }) - }); + return Spacebars.include(self.lookupTemplate("foo"), UI.block(function() { + var self = this; + return "abc"; + })); }); run("{{#if cond}}aaa{{else}}bbb{{/if}}", @@ -131,89 +128,88 @@ Tinytest.add("spacebars - compiler output", function (test) { run("{{> foo bar}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return Spacebars.call(self.lookup("bar")); - }); + return UI.With(function() { + return Spacebars.call(self.lookup("bar")); + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo")); + })); }); run("{{> foo x=bar}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return { x: Spacebars.call(self.lookup("bar")) }; - }); + return UI.With(function() { + return { + x: Spacebars.call(self.lookup("bar")) + }; + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo")); + })); }); run("{{> foo bar.baz}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")); - }); + return UI.With(function() { + return Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")); + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo")); + })); }); run("{{> foo x=bar.baz}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return { - x: Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")) - }; - }); + return UI.With(function() { + return { + x: Spacebars.call(Spacebars.dot(self.lookup("bar"), "baz")) + }; + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo")); + })); }); run("{{> foo bar baz}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return Spacebars.dataMustache(self.lookup("bar"), - self.lookup("baz")); - }); + return UI.With(function() { + return Spacebars.dataMustache(self.lookup("bar"), self.lookup("baz")); + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo")); + })); }); run("{{#foo bar baz}}aaa{{/foo}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return Spacebars.dataMustache(self.lookup("bar"), - self.lookup("baz")); - }, - { - content: UI.block(function () { - var self = this; - return "aaa"; - }) - }); + return UI.With(function() { + return Spacebars.dataMustache(self.lookup("bar"), self.lookup("baz")); + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo"), UI.block(function() { + var self = this; + return "aaa"; + })); + })); }); run("{{#foo p.q r.s}}aaa{{/foo}}", function() { var self = this; - return Spacebars.include( - self.lookup("foo", {template: true}), - function () { - return Spacebars.dataMustache( - Spacebars.dot(self.lookup("p"), "q"), - Spacebars.dot(self.lookup("r"), "s")); - }, - { - content: UI.block(function () { - var self = this; - return "aaa"; - }) - }); + return UI.With(function() { + return Spacebars.dataMustache(Spacebars.dot(self.lookup("p"), "q"), Spacebars.dot(self.lookup("r"), "s")); + }, UI.block(function() { + var self = this; + return Spacebars.include(self.lookupTemplate("foo"), UI.block(function() { + var self = this; + return "aaa"; + })); + })); }); run("", diff --git a/packages/spacebars-compiler/spacebars-compiler.js b/packages/spacebars-compiler/spacebars-compiler.js index ffc78660bb..313f32bc8e 100644 --- a/packages/spacebars-compiler/spacebars-compiler.js +++ b/packages/spacebars-compiler/spacebars-compiler.js @@ -190,10 +190,10 @@ var codeGenTemplateTag = function (tag) { if (! tag.args.length) throw new Error("#" + path[0] + " requires an argument"); - var info = codeGenInclusionArgs(tag); - var dataFunc = info.dataFuncCode; // must exist (tag.args.length > 0) - var contentBlock = info.extraArgs.content; // must exist - var elseContentBlock = info.extraArgs.elseContent; // may not exist + var codeParts = codeGenInclusionParts(tag); + var dataFunc = codeParts.dataFunc; // must exist (tag.args.length > 0) + var contentBlock = codeParts.content; // must exist + var elseContentBlock = codeParts.elseContent; // may not exist var callArgs = [dataFunc, contentBlock]; if (elseContentBlock) @@ -210,18 +210,28 @@ var codeGenTemplateTag = function (tag) { compCode = 'function () { return ' + compCode + '; }'; } - var argGen = codeGenInclusionArgs(tag); - var dataFuncCode = argGen.dataFuncCode; - var extraArgs = argGen.extraArgs; + var codeParts = codeGenInclusionParts(tag); + var dataFunc = codeParts.dataFunc; + var content = codeParts.content; + var elseContent = codeParts.elseContent; var includeArgs = [compCode]; - if (dataFuncCode || extraArgs) - includeArgs.push(dataFuncCode || 'null'); - if (extraArgs) - includeArgs.push(makeObjectLiteral(extraArgs)); + if (content) { + includeArgs.push(content); + if (elseContent) + includeArgs.push(elseContent); + } - return HTML.EmitCode( + var includeCode = HTML.EmitCode( 'Spacebars.include(' + includeArgs.join(', ') + ')'); + + if (dataFunc) { + includeCode = HTML.EmitCode( + 'UI.With(' + dataFunc + ', UI.block(' + + Spacebars.codeGen(includeCode) + '))'); + } + + return includeCode; } } else { // Can't get here; TemplateTag validation should catch any @@ -264,9 +274,10 @@ var codeGenPath = function (path, opts) { } var args = [toJSLiteral(path[0])]; + var lookupMethod = 'lookup'; if (opts && opts.lookupTemplate && path.length === 1) - args.push('{template: true}'); - var code = 'self.lookup(' + args.join(', ') + ')'; + lookupMethod = 'lookupTemplate'; + var code = 'self.' + lookupMethod + '(' + args.join(', ') + ')'; if (path.length > 1) { code = 'Spacebars.dot(' + code + ', ' + @@ -346,33 +357,32 @@ var codeGenMustacheArgs = function (tagArgs) { return args; }; -// Returns an object containing two properties, both optional -// (in which case they may be absent or `null`). -// These properties are for code generation of the second and -// third arguments to `Spacebars.include`. +// Takes an inclusion tag and returns an object containing these properties, +// all optional, whose values are JS source code: // -// - `dataFuncCode` - source code of a data function literal -// - `extraArgs` - map of key (e.g. "content") to source code -var codeGenInclusionArgs = function (tag) { - var extraArgs = null; // [source] +// - `dataFunc` - source code of a data function literal +// - `content` - source code of a content block +// - `elseContent` - source code of an elseContent block +// +// Implements the calling convention for inclusions. +var codeGenInclusionParts = function (tag) { + var ret = {}; if ('content' in tag) { - extraArgs = (extraArgs || {}); - extraArgs.content = ( + ret.content = ( 'UI.block(' + Spacebars.codeGen(tag.content) + ')'); } if ('elseContent' in tag) { - extraArgs = (extraArgs || {}); - extraArgs.elseContent = ( + ret.elseContent = ( 'UI.block(' + Spacebars.codeGen(tag.elseContent) + ')'); } - var dataFuncCode = null; // source (exclusive of `function () { ...`) + var dataFuncCode = null; var args = tag.args; if (! args.length) { // e.g. `{{#foo}}` - return { extraArgs: extraArgs }; + return ret; } else if (args[0].length === 3) { // keyword arguments only, e.g. `{{> point x=1 y=2}}` var dataProps = {}; @@ -395,10 +405,9 @@ var codeGenInclusionArgs = function (tag) { 'dataMustache'); } - dataFuncCode = 'function () { return ' + dataFuncCode + '; }'; + ret.dataFunc = 'function () { return ' + dataFuncCode + '; }'; - return { dataFuncCode: dataFuncCode, - extraArgs: extraArgs }; + return ret; }; diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 1b0b38994f..ab617cd50d 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -288,8 +288,8 @@ Tinytest.add("spacebars - templates - block helper function with one helper arg" var tmpl = Template.spacebars_template_test_block_helper_function_one_helper_arg; var R = ReactiveVar("bar"); tmpl.bar = function () { return R.get(); }; - tmpl.foo = function (x) { - if (x === "bar") + tmpl.foo = function () { + if (String(this) === "bar") return Template.spacebars_template_test_content; else return null; diff --git a/packages/spacebars/spacebars-runtime.js b/packages/spacebars/spacebars-runtime.js index 49dc43bfcf..fdc460e8e1 100644 --- a/packages/spacebars/spacebars-runtime.js +++ b/packages/spacebars/spacebars-runtime.js @@ -1,56 +1,40 @@ Spacebars = {}; -// * `templateOrFunction` - template (component) or function returning one -// * `dataFunc` - (optional) function returning data context -// * `extraArgs` - (optional) dictionary that may have `content`/`elseContent` -Spacebars.include = function (templateOrFunction, dataFunc, extraArgs) { - var result = templateOrFunction; +// * `templateOrFunction` - template (component) or function returning a template +// or null +Spacebars.include = function (templateOrFunction, contentBlock, elseContentBlock) { + if (contentBlock && ! UI.isComponent(contentBlock)) + throw new Error('Second argument to Spacebars.include must be a template or UI.block if present'); + if (elseContentBlock && ! UI.isComponent(elseContentBlock)) + throw new Error('Third argument to Spacebars.include must be a template or UI.block if present'); - var underscoredArgs; - if (extraArgs) { - underscoredArgs = {}; - for (var k in extraArgs) - underscoredArgs['__'+k] = extraArgs[k]; + var props = null; + if (contentBlock) { + props = (props || {}); + props.__content = contentBlock; + } + if (elseContentBlock) { + props = (props || {}); + props.__elseContent = elseContentBlock; } - // extend `result` with `underscoredArgs`, whether or not it's a function. - // if it's a function, validate and isolate the result. - if (typeof result === 'function') { - result = function () { - var resultTmpl = Deps.isolateValue(function () { - var ret = templateOrFunction(); - if (ret != null && ! UI.isComponent(ret)) - throw new Error("Expected null or template in return value from helper, found: " + ret); - return ret; - }); - if (extraArgs) { - resultTmpl = (resultTmpl ? resultTmpl.extend(underscoredArgs) : - resultTmpl); - } - return resultTmpl; - }; - } else { - if (result != null && ! UI.isComponent(result)) - throw new Error("Expected null or template in return value from helper, found: " + result); - if (extraArgs) - result = result ? result.extend(underscoredArgs) : result; - } + if (UI.isComponent(templateOrFunction)) + return templateOrFunction.extend(props); - if (dataFunc) { - if (typeof dataFunc !== 'function') - throw new Error("Second argument to Spacebars.include must be a function"); + var func = templateOrFunction; - if (typeof result === 'function') { - var func = result; - result = UI.block(function () { return func; }); - } - return UI.With(dataFunc, result); - } else { - return result; - } + return function () { + var tmpl = Deps.isolateValue(func); + + if (tmpl === null) + return null; + if (! UI.isComponent(tmpl)) + throw new Error("Expected null or template in return value from inclusion function, found: " + tmpl); + + return tmpl.extend(props); + }; }; - // Executes `{{foo bar baz}}` when called on `(foo, bar, baz)`. // If `bar` and `baz` are functions, they are called before // `foo` is called on them. diff --git a/packages/ui/fields.js b/packages/ui/fields.js index d71e5bdf06..2ff1c9979e 100644 --- a/packages/ui/fields.js +++ b/packages/ui/fields.js @@ -114,6 +114,9 @@ _extend(UI.Component, { return result; }; }, + lookupTemplate: function (id) { + return this.lookup(id, {template: true}); + }, get: function (id) { // support `this.get()` to get the data context. if (id === undefined)