From a9c4dfaecd22e29a0d7c8173f6b16adfd51ec332 Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Fri, 24 Jan 2014 13:22:02 -0800 Subject: [PATCH] Implement new # and > calling convention Both {{#foo}} and {{> foo}} are handled as follows: - foo may be a template or a function returning a template - if there are only keyword arguments, they are used to create a data context object (e.g. `x=1 y=2` becomes `{x:1, y:2}`) - if there are any positional arguments, the arguments are treated as a nested helper call, meaning that the first one will be called on the rest (including keyword arguments) if it is a function Rendering-wise, the template should only be re-rendered if foo is a function and it is invalidated to return a different value (though the "isolate" that would compare the new value to the old is not on this branch at the moment). This change also makes the built-in block helpers (#if, #each, etc.) be functions (render macros) instead of components (though #each is still implemented with a component). This avoids data context issues with #if and #unless (the new calling convention isn't really designed to support making something like #if). It also should make generated code cleaner to call `UI.If` instead of including it as a component. --- .../spacebars-compiler/spacebars-compiler.js | 297 ++++++++++-------- packages/spacebars-compiler/templatetag.js | 15 +- packages/spacebars/spacebars-runtime.js | 92 +++--- packages/ui/builtins.js | 81 +++++ packages/ui/components.js | 78 ----- packages/ui/each.js | 12 +- packages/ui/package.js | 2 +- 7 files changed, 294 insertions(+), 283 deletions(-) create mode 100644 packages/ui/builtins.js delete mode 100644 packages/ui/components.js diff --git a/packages/spacebars-compiler/spacebars-compiler.js b/packages/spacebars-compiler/spacebars-compiler.js index f90923f821..5d3c3ec28b 100644 --- a/packages/spacebars-compiler/spacebars-compiler.js +++ b/packages/spacebars-compiler/spacebars-compiler.js @@ -143,36 +143,64 @@ var optimize = function (tree) { // ============================================================ // Code-generation of template tags -var builtInComponents = { - 'content': '__content', - 'elseContent': '__elseContent', +var builtInBlockHelpers = { 'if': 'UI.If', 'unless': 'UI.Unless', - 'with': 'UI.With', + 'with': 'Spacebars.With', 'each': 'UI.Each' }; +var builtInLexicals = { + 'content': 'template.__content', + 'elseContent': 'template.__elseContent' +}; + var codeGenTemplateTag = function (tag) { if (tag.position === HTML.TEMPLATE_TAG_POSITION.IN_START_TAG) { // only `tag.type === 'DOUBLE'` allowed (by earlier validation) return HTML.EmitCode('function () { return ' + - codeGenMustache(tag, 'attrMustache') + '; }'); + codeGenMustache(tag.path, tag.args, 'attrMustache') + + '; }'); } else { if (tag.type === 'DOUBLE') { return HTML.EmitCode('function () { return ' + - codeGenMustache(tag) + '; }'); + codeGenMustache(tag.path, tag.args) + '; }'); } else if (tag.type === 'TRIPLE') { return HTML.EmitCode('function () { return Spacebars.makeRaw(' + - codeGenMustache(tag) + '); }'); + codeGenMustache(tag.path, tag.args) + '); }'); } else if (tag.type === 'INCLUSION' || tag.type === 'BLOCKOPEN') { var path = tag.path; - var compCode = codeGenPath(path); - if (path.length === 1) { - var compName = path[0]; - if (builtInComponents.hasOwnProperty(compName)) { - compCode = builtInComponents[compName]; - } else { + if (tag.type === 'BLOCKOPEN' && + builtInBlockHelpers.hasOwnProperty(path[0])) { + // if, unless, with, each. + // + // If someone tries to do `{{> if}}`, we don't + // get here, but an error is thrown when we try to codegen the path. + + // Note: If we caught these errors earlier, while scanning, we'd be able to + // provide nice line numbers. + if (path.length > 1) + throw new Error("Unexpected dotted path beginning with " + path[0]); + 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 callArgs = [dataFunc, contentBlock]; + if (elseContentBlock) + callArgs.push(elseContentBlock); + + return HTML.EmitCode( + builtInBlockHelpers[path[0]] + '(' + callArgs.join(', ') + ')'); + + } else { + var compCode = codeGenPath(path); + + if (path.length === 1) { // toObjectLiteralKey returns `"foo"` or `foo` depending on // whether `foo` is a safe JavaScript identifier. var member = toObjectLiteralKey(path[0]); @@ -180,15 +208,24 @@ var codeGenTemplateTag = function (tag) { 'Template[' + member + ']' : 'Template.' + member); compCode = ('(' + templateDotFoo + ' || ' + compCode + ')'); + } else { + // path code may be reactive; wrap it + compCode = 'function () { return ' + compCode + '; }'; } + + var argGen = codeGenInclusionArgs(tag); + var dataFuncCode = argGen.dataFuncCode; + var extraArgs = argGen.extraArgs; + + var includeArgs = [compCode]; + if (dataFuncCode || extraArgs) + includeArgs.push(dataFuncCode || 'null'); + if (extraArgs) + includeArgs.push(makeObjectLiteral(extraArgs)); + + return HTML.EmitCode( + 'Spacebars.include(' + includeArgs.join(', ') + ')'); } - - var includeArgs = codeGenInclusionArgs(tag); - - return HTML.EmitCode( - 'function () { return Spacebars.include(' + compCode + - (includeArgs.length ? ', ' + includeArgs.join(', ') : '') + - '); }'); } else { // Can't get here; TemplateTag validation should catch any // inappropriate tag types that might come out of the parser. @@ -204,97 +241,6 @@ var makeObjectLiteral = function (obj) { return '{' + parts.join(', ') + '}'; }; - -var codeGenInclusionArgs = function (tag) { - var args = null; - var posArgs = []; - - if ('content' in tag) { - args = (args || {}); - args.__content = ( - 'UI.block(' + Spacebars.codeGen(tag.content) + ')'); - } - if ('elseContent' in tag) { - args = (args || {}); - args.__elseContent = ( - 'UI.block(' + Spacebars.codeGen(tag.elseContent) + ')'); - } - - // precalculate the number of positional args - var numPosArgs = 0; - _.each(tag.args, function (arg) { - if (arg.length === 2) - numPosArgs++; - }); - - _.each(tag.args, function (arg) { - var argType = arg[0]; - var argValue = arg[1]; - - var isKeyword = (arg.length > 2); - - var argCode; - switch (argType) { - case 'STRING': - case 'NUMBER': - case 'BOOLEAN': - case 'NULL': - argCode = toJSLiteral(argValue); - break; - case 'PATH': - var path = argValue; - argCode = codeGenPath(path); - // a single-segment path will compile to something like - // `self.lookup("foo")` which never establishes any dependencies, - // while `Spacebars.dot(self.lookup("foo"), "bar")` may establish - // dependencies. - // - // In the multi-positional-arg construct, don't wrap pos args here. - if (! ((path.length === 1) || (numPosArgs > 1))) - argCode = 'function () { return Spacebars.call(' + argCode + '); }'; - break; - default: - // can't get here - throw new Error("Unexpected arg type: " + argType); - } - - if (isKeyword) { - // keyword argument (represented as [type, value, name]) - var name = arg[2]; - args = (args || {}); - args[name] = argCode; - } else { - // positional argument - posArgs.push(argCode); - } - }); - - if (posArgs.length === 1) { - args = (args || {}); - args.data = posArgs[0]; - } else if (posArgs.length > 1) { - // only allowed for block helper (which has already been - // checked at parse time); call first - // argument as a function on the others - args = (args || {}); - args.data = 'function () { return Spacebars.call(' + posArgs.join(', ') + '); }'; - } - - if (args) - return [makeObjectLiteral(args)]; - - return []; -}; - -var codeGenMustache = function (tag, mustacheType) { - var nameCode = codeGenPath(tag.path); - var argCode = codeGenArgs(tag.args); - var mustache = (mustacheType || 'mustache'); - - return 'Spacebars.' + mustache + '(' + nameCode + - (argCode ? ', ' + argCode.join(', ') : '') + ')'; -}; - // `path` is an array of at least one string. // // If `path.length > 1`, the generated code may be reactive @@ -302,12 +248,15 @@ var codeGenMustache = function (tag, mustacheType) { // // No code is generated to call the result if it's a function. var codeGenPath = function (path) { - // Let {{#if content}} check whether this template was invoked via - // inclusion or as a block helper. - if (builtInComponents.hasOwnProperty(path[0])) { + if (builtInBlockHelpers.hasOwnProperty(path[0])) + throw new Error("Can't use the built-in '" + path[0] + "' here"); + // Let `{{#if content}}` check whether this template was invoked via + // inclusion or as a block helper, in addition to supporting + // `{{> content}}`. + if (builtInLexicals.hasOwnProperty(path[0])) { if (path.length > 1) throw new Error("Unexpected dotted path beginning with " + path[0]); - return builtInComponents[path[0]]; + return builtInLexicals[path[0]]; } var code = 'self.lookup(' + toJSLiteral(path[0]) + ')'; @@ -320,31 +269,55 @@ var codeGenPath = function (path) { return code; }; +// Generates code for an `[argType, argValue]` argument spec, +// ignoring the third element (keyword argument name) if present. +// +// The resulting code may be reactive (in the case of a PATH of +// more than one element) and is not wrapped in a closure. +var codeGenArgValue = function (arg) { + var argType = arg[0]; + var argValue = arg[1]; + + var argCode; + switch (argType) { + case 'STRING': + case 'NUMBER': + case 'BOOLEAN': + case 'NULL': + argCode = toJSLiteral(argValue); + break; + case 'PATH': + argCode = codeGenPath(argValue); + break; + default: + // can't get here + throw new Error("Unexpected arg type: " + argType); + } + + return argCode; +}; + +// Generates a call to `Spacebars.fooMustache` on evaluated arguments. +// The resulting code has no function literals and must be wrapped in +// one for fine-grained reactivity. +var codeGenMustache = function (path, args, mustacheType) { + var nameCode = codeGenPath(path); + var argCode = codeGenMustacheArgs(args); + var mustache = (mustacheType || 'mustache'); + + return 'Spacebars.' + mustache + '(' + nameCode + + (argCode ? ', ' + argCode.join(', ') : '') + ')'; +}; + // returns: array of source strings, or null if no // args at all. -var codeGenArgs = function (tagArgs) { +var codeGenMustacheArgs = function (tagArgs) { var kwArgs = null; // source -> source var args = null; // [source] + // tagArgs may be null _.each(tagArgs, function (arg) { - var argType = arg[0]; - var argValue = arg[1]; - - var argCode; - switch (argType) { - case 'STRING': - case 'NUMBER': - case 'BOOLEAN': - case 'NULL': - argCode = toJSLiteral(argValue); - break; - case 'PATH': - argCode = codeGenPath(argValue); - break; - default: - // can't get here - throw new Error("Unexpected arg type: " + argType); - } + var argCode = codeGenArgValue(arg); if (arg.length > 2) { // keyword argument (represented as [type, value, name]) @@ -366,6 +339,59 @@ var codeGenArgs = 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`. +// +// - `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] + + if ('content' in tag) { + extraArgs = (extraArgs || {}); + extraArgs.content = ( + 'UI.block(' + Spacebars.codeGen(tag.content) + ')'); + } + if ('elseContent' in tag) { + extraArgs = (extraArgs || {}); + extraArgs.elseContent = ( + 'UI.block(' + Spacebars.codeGen(tag.elseContent) + ')'); + } + + var dataFuncCode = null; // source (exclusive of `function () { ...`) + + var args = tag.args; + if (! args.length) { + // e.g. `{{#foo}}` + return { extraArgs: extraArgs }; + } else if (args[0].length === 3) { + // keyword arguments only, e.g. `{{> point x=1 y=2}}` + var args = {}; + _.each(args, function (arg) { + var argKey = arg[2]; + args[argKey] = 'Spacebars.call(' + codeGenArgValue(arg) + ')'; + }); + dataFuncCode = makeObjectLiteral(args); + } else if (args[0][0] !== 'PATH') { + // literal first argument, e.g. `{{> foo "blah"}}` + // + // tag validation has confirmed, in this case, that there is only + // one argument (`args.length === 1`) + dataFuncCode = codeGenArgValue(args[0]); + } else { + dataFuncCode = codeGenMustache(args[0][1], args.slice(1), + 'dataMustache'); + } + + dataFuncCode = 'function () { return ' + dataFuncCode + '; }'; + + return { dataFuncCode: dataFuncCode, + extraArgs: extraArgs }; +}; + + // ============================================================ // Main compiler @@ -430,8 +456,7 @@ Spacebars.codeGen = function (parseTree, options) { // support `{{> content}}` and `{{> elseContent}}` with // lexical scope by creating a local variable in the // template's render function. - code += 'var __content = self.__content, ' + - '__elseContent = self.__elseContent; '; + code += 'var template = this; '; } code += 'return '; code += HTML.toJS(tree); diff --git a/packages/spacebars-compiler/templatetag.js b/packages/spacebars-compiler/templatetag.js index b0ec8a40ec..99307e7bd0 100644 --- a/packages/spacebars-compiler/templatetag.js +++ b/packages/spacebars-compiler/templatetag.js @@ -399,15 +399,14 @@ var isAtBlockCloseOrElse = function (scanner) { // nothing. var validateTag = function (ttag, scanner) { - if (ttag.type === 'INCLUSION') { - // throw error on >1 positional arguments - var numPosArgs = 0; + if (ttag.type === 'INCLUSION' || ttag.type === 'BLOCKOPEN') { var args = ttag.args; - for (var i = 0; i < args.length; i++) - if (args[i].length === 2) - numPosArgs++; - if (numPosArgs > 1) - scanner.fatal("Only one positional argument is allowed in {{> ... }}"); + if (args.length > 1 && args[0].length === 2 && args[0][0] !== 'PATH') { + // we have a positional argument that is not a PATH followed by + // other arguments + scanner.fatal("Can't have a " + args[0][0] + " argument here followed " + + "by other arguments"); + } } var position = ttag.position || HTML.TEMPLATE_TAG_POSITION.ELEMENT; diff --git a/packages/spacebars/spacebars-runtime.js b/packages/spacebars/spacebars-runtime.js index 6ad8233f87..d09cd5fa69 100644 --- a/packages/spacebars/spacebars-runtime.js +++ b/packages/spacebars/spacebars-runtime.js @@ -12,63 +12,41 @@ var safeEquals = function (a, b) { (typeof a === 'string')); }; -Spacebars.include = function (kindOrFunc, args) { - args = args || {}; - if (typeof kindOrFunc === 'function') { - // function block helper - var func = kindOrFunc; +// * `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; - var hash = {}; - // Call arguments if they are functions. This may cause - // reactive dependencies! - for (var k in args) { - if (k !== 'data') { - var v = args[k]; - hash[k] = (typeof v === 'function' ? v() : v); - } - } + if (extraArgs) { + var underscoredArgs = {}; + for (var k in extraArgs) + underscoredArgs['__'+k] = extraArgs[k]; - var result; - if ('data' in args) { - var data = args.data; - data = (typeof data === 'function' ? data() : data); - result = func(data, { hash: hash }); + // extend `result` with `underscoredArgs`, whether or not it's a function + if (typeof result === 'function') { + result = function () { + // todo: isolate the calculation of `templateOrFunction` + var result = templateOrFunction(); + result = result.extend(underscoredArgs); + return result; + }; } else { - result = func({ hash: hash }); + result = result.extend(underscoredArgs); } - // In `{{#foo}}...{{/foo}}`, if `foo` is a function that - // returns a component, attach __content and __elseContent - // to it. - if (UI.isComponent(result) && - (('__content' in args) || ('__elseContent' in args))) { - var extra = {}; - if ('__content' in args) - extra.__content = args.__content; - if ('__elseContent' in args) - extra.__elseContent = args.__elseContent; - result = result.extend(extra); + } + + if (dataFunc) { + if (typeof dataFunc !== 'function') + throw new Error("Second argument to Spacebars.include must be a function"); + + if (typeof result === 'function') { + var func = result; + result = UI.block(function () { return func; }); } - return result; + return UI.With(UI.emboxValue(dataFunc, safeEquals), result); } else { - // Component - var kind = kindOrFunc; - if (! UI.isComponent(kind)) - throw new Error("Expected template, found: " + kind); - - // Note that there are no reactive dependencies established here. - if (args) { - var emboxedArgs = {}; - for (var k in args) { - if (k === '__content' || k === '__elseContent') - emboxedArgs[k] = args[k]; - else - emboxedArgs[k] = UI.emboxValue(args[k], safeEquals); - } - - return kind.extend(emboxedArgs); - } else { - return kind; - } + return result; } }; @@ -133,6 +111,12 @@ Spacebars.attrMustache = function (value/*, args*/) { } }; +Spacebars.dataMustache = function (value/*, args*/) { + var result = Spacebars.mustacheImpl.apply(null, arguments); + + return result; +}; + // Idempotently wrap in `HTML.Raw`. // // Called on the return value from `Spacebars.mustache` in case the @@ -221,3 +205,9 @@ Spacebars.dot = function (value, id1/*, id2, ...*/) { return result.apply(value, arguments); }; }; + +// Implement Spacebars's #with, which renders its else case (or nothing) +// if the argument is falsy. +Spacebars.With = function (argFunc, contentBlock, elseContentBlock) { + return UI.If(argFunc, UI.With(argFunc, contentBlock), elseContentBlock); +}; diff --git a/packages/ui/builtins.js b/packages/ui/builtins.js new file mode 100644 index 0000000000..a8dde04125 --- /dev/null +++ b/packages/ui/builtins.js @@ -0,0 +1,81 @@ + +UI.If = function (argFunc, contentBlock, elseContentBlock) { + checkBlockHelperArguments('If', argFunc, contentBlock, elseContentBlock); + + return function () { + if (getCondition(argFunc)) + return contentBlock; + else + return elseContentBlock || null; + }; +}; + + +UI.Unless = function (argFunc, contentBlock, elseContentBlock) { + checkBlockHelperArguments('Unless', argFunc, contentBlock, elseContentBlock); + + return function () { + if (! getCondition(argFunc)) + return contentBlock; + else + return elseContentBlock || null; + }; +}; + +// Unlike Spacebars.With, there's no else case and no conditional logic. +// +// We don't do any reactive emboxing of `argFunc` here; it should be done +// by the caller if efficiency and/or number of calls to the data source +// is important. +UI.With = function (argFunc, contentBlock) { + checkBlockHelperArguments('With', argFunc, contentBlock); + + var block = UI.block(function () { + return contentBlock; + }); + block.data = argFunc; + + return block; +}; + +UI.Each = function (argFunc, contentBlock, elseContentBlock) { + checkBlockHelperArguments('Each', argFunc, contentBlock, elseContentBlock); + + return UI.EachImpl.extend({ + __sequence: argFunc, + __content: contentBlock, + __elseContent: elseContentBlock + }); +}; + +var checkBlockHelperArguments = function (which, argFunc, contentBlock, elseContentBlock) { + if (typeof argFunc !== 'function') + throw new Error('First argument to ' + which + ' must be a function'); + if (! UI.isComponent(contentBlock)) + throw new Error('Second argument to ' + which + ' must be a template or UI.block'); + if (elseContentBlock && ! UI.isComponent(elseContentBlock)) + throw new Error('Third argument to ' + which + ' must be a template or UI.block if present'); +}; + +// Acts like `!! conditionFunc()` except: +// +// - Empty array is considered falsy +// - The result is Deps.isolated (doesn't trigger invalidation +// as long as the condition stays truthy or stays falsy +var getCondition = function (conditionFunc) { + return Deps.isolateValue(function () { + // `condition` is emboxed; it is always a function, + // and it only triggers invalidation if its return + // value actually changes. We still need to isolate + // the calculation of whether it is truthy or falsy + // in order to not re-render if it changes from one + // truthy or falsy value to another. + var cond = conditionFunc(); + + // empty arrays are treated as falsey values + if (cond instanceof Array && cond.length === 0) + return false; + else + return !! cond; + }); +}; diff --git a/packages/ui/components.js b/packages/ui/components.js deleted file mode 100644 index 9ffff780e9..0000000000 --- a/packages/ui/components.js +++ /dev/null @@ -1,78 +0,0 @@ - -UI.If = Component.extend({ - kind: 'If', - init: function () { - // XXX this probably deserves a better explanation if this code is - // going to stay with us. - this.condition = this.data; - - // content doesn't see the condition as `data` - this.data = undefined; - // XXX I guess this means it's kosher to mutate properties - // of a Component during init (but presumably not before - // or after)? - }, - render: function () { - var self = this; - return function () { - var condition = getCondition(self); - - // `__content` and `__elseContent` are passed by - // the compiler and are *not* emboxed, they are just - // Component kinds. - return condition ? self.__content : self.__elseContent; - }; - } -}); - -// Acts like `!! self.condition()` except: -// -// - Empty array is considered falsy -// - The result is Deps.isolated (doesn't trigger invalidation -// as long as the condition stays truthy or stays falsy -var getCondition = function (self) { - return Deps.isolateValue(function () { - // `condition` is emboxed; it is always a function, - // and it only triggers invalidation if its return - // value actually changes. We still need to isolate - // the calculation of whether it is truthy or falsy - // in order to not re-render if it changes from one - // truthy or falsy value to another. - var cond = self.condition(); - - // empty arrays are treated as falsey values - if (cond instanceof Array && cond.length === 0) - return false; - else - return !! cond; - }); -}; - -UI.Unless = Component.extend({ - kind: 'Unless', - init: function () { - this.condition = this.data; - this.data = undefined; - }, - render: function () { - var self = this; - return function () { - var condition = getCondition(self); - return (! condition) ? self.__content : self.__elseContent; - }; - } -}); - -UI.With = Component.extend({ - kind: 'With', - init: function () { - this.condition = this.data; - }, - render: function () { - var self = this; - return function () { - var condition = getCondition(self); - return condition ? self.__content : self.__elseContent; - }; - } -}); diff --git a/packages/ui/each.js b/packages/ui/each.js index 1d967d6515..5a0976f1ad 100644 --- a/packages/ui/each.js +++ b/packages/ui/each.js @@ -1,11 +1,5 @@ -UI.Each = Component.extend({ +UI.EachImpl = Component.extend({ typeName: 'Each', - init: function () { - // don't keep `this.data` around so that `{{..}}` skips over this - // component - this.sequence = this.data; - this.data = undefined; - }, render: function (modeHint) { var self = this; var content = self.__content; @@ -27,7 +21,7 @@ UI.Each = Component.extend({ // a method like component.populate(domRange) and one // like renderStatic() or even renderHTML / renderText. var parts = _.map( - ObserveSequence.fetch(self.get('sequence')), + ObserveSequence.fetch(self.__sequence()), function (item) { return content.withData(function () { return item; @@ -75,7 +69,7 @@ UI.Each = Component.extend({ try { this.observeHandle = ObserveSequence.observe(function () { - return self.get('sequence'); + return self.__sequence(); }, { addedAt: function (id, item, i, beforeId) { addToCount(1); diff --git a/packages/ui/package.js b/packages/ui/package.js index c31fdca5f0..561c74f3dc 100644 --- a/packages/ui/package.js +++ b/packages/ui/package.js @@ -23,7 +23,7 @@ Package.on_use(function (api) { api.add_files(['attrs.js', 'render.js', - 'components.js', + 'builtins.js', 'each.js', 'fields.js' ]);