Fix dots in mustaches by rewriting Spacebars.index

Also comment out failing old tests in "spacebars" package

packages/spacebars-tests/template_tests.js

@@ -177,12 +177,26 @@ Tinytest.add("spacebars - templates - inclusion dotted args", function (test) {
   // `{{> foo bar.baz}}`
   var tmpl = Template.spacebars_template_test_inclusion_dotted_args;
 
-  // XXX
+  var initCount = 0;
+  tmpl.foo = Template.spacebars_template_test_bracketed_this.extend({
+    init: function () { initCount++; }
+  });
+  var R = ReactiveVar('david');
+  tmpl.bar = function () {
+    // make sure `this` is bound correctly
+    return { baz: this.symbol + R.get() };
+  };
 
-  // This test should fail when `foo` is `bracketed_this` and `bar` is
-  // a function by detecting that when the return value of `bar` changes
-  // reactively, the whole `bracketed_this` is re-rendered even though
-  // a `data` change shouldn't cause that.  Or something.
+  var div = renderToDiv(tmpl.withData({symbol:'%'}));
+  test.equal(initCount, 1);
+  test.equal(div.innerHTML, '[%david]');
+
+  R.set('avi');
+  Deps.flush();
+  test.equal(div.innerHTML, '[%avi]');
+  // check that invalidating the argument to `foo` doesn't require
+  // creating a new `foo`.
+  test.equal(initCount, 1);
 });
 
 Tinytest.add("spacebars - templates - block helper", function (test) {

packages/spacebars/spacebars.js

@@ -934,44 +934,59 @@ Spacebars.compile = function (inputString, options) {
   return tokensToRenderFunc(tree.bodyTokens);
 };
 
-// `Spacebars.index(foo, "bar", "baz")` achieves a special kind of
-// `foo.bar.baz` used to implement dotted paths in templates:
+// `Spacebars.index(foo, "bar", "baz")` performs a special kind
+// of `foo.bar.baz` that allows safe indexing of `null` and
+// indexing of functions to get other functions.
 //
-// - Indexing a falsy thing just returns the thing (e.g. undefined)
-// - Indexing a function calls the function
-// - Functions that result from indexing have a bound value of `this`.
-//   In JavaScript, `x = foo.bar; x()` won't pass `foo` as `this` in `x`,
-//   but `x = Spacebars.index(foo, "bar"); x()` will call a wrapped
-//   version of the function `foo.bar` that always substitutes
-//   `foo` for `this`.
-Spacebars.index = function (value/*, identifiers*/) {
-  var identifiers = Array.prototype.slice.call(arguments, 1);
-
-  // The object we got `curValue` from by indexing.
-  // For the value itself, we don't know the appropriate value
-  // of `this`, so we assume it is already bound.
-  var nextThis = null;
-
-  _.each(identifiers, function (id) {
-    if (typeof value === 'function') {
-      // Call a getter -- in `{{foo.bar}}`, call `foo()` if it
-      // is a function before indexing it with `bar`.
-      value = value.call(nextThis);
-    }
-    nextThis = value;
-    // support "soft dot" where if `foo` doesn't exist, you can
-    // still do `{{foo.bar.baz}}`.
-    if (value)
-      value = value[id];
-  });
-
-  if (typeof value === 'function') {
-    // bind `this` for resulting function, so it can be
-    // called with `Spacebars.call`.
-    value = _.bind(value, nextThis);
+// `foo` must be one of three types of values, and the return value
+// is also one of these three types:
+// * Falsy values
+// * Non-functions (i.e. constants)
+// * Fully "bound" functions, which take no arguments and ignore `this`
+//
+// `Spacebars.index("foo", bar)` behaves as follows:
+//
+// * If `foo` is falsy, `foo` is returned.
+//
+// * If either `foo` is a function or `foo.bar` is, then a new
+// fully-bound function is returned that, when called, will calculate
+// a "safe" version of `foo().bar()`, where "dot" on a falsy value
+// just returns the falsy value, and function calls are a no-op on
+// non-functions.
+//
+// * Otherwise, the non-function `foo.bar` is returned.
+Spacebars.index = function (value, id1/*, id2, ...*/) {
+  if (arguments.length > 2) {
+    // Note: doing this recursively is probably less efficient than
+    // doing it in an iterative loop.
+    var argsForRecurse = [];
+    argsForRecurse.push(Spacebars.index(value, id1));
+    argsForRecurse.push.apply(argsForRecurse,
+                              Array.prototype.slice.call(arguments, 2));
+    return Spacebars.index.apply(null, argsForRecurse);
   }
 
-  return value;
+  if (! value)
+    return value; // falsy, don't index, pass through
+
+  if (typeof value !== 'function') {
+    var result = value[id1];
+    if (typeof result !== 'function')
+      return result;
+    return function () {
+      return result.call(value);
+    };
+  }
+
+  return function () {
+    var foo = value();
+    if (! foo)
+      return foo; // falsy, don't index, pass through
+    var bar = foo[id1];
+    if (typeof bar !== 'function')
+      return bar;
+    return bar.call(foo);
+  };
 };
 
 Spacebars.call = function (value/*, args*/) {

packages/spacebars/spacebars_tests.js

@@ -360,6 +360,12 @@ Tinytest.add("spacebars - compiler", function (test) {
       '    ">abc</a>");',
       '}');
 
+  // NOTE: These are old tests of code generation from various previous versions
+  // of the compiler.  Once the form of generated code stabilizes, it would be
+  // nice to have these tests as a way of seeing that code generation is working
+  // as intended and pretty-printing remains correct, as well as as a form of
+  // documentation.
+  /*
   run('<a foo={{bar}}>',
 
       'function (buf) {',
@@ -457,4 +463,62 @@ Tinytest.add("spacebars - compiler", function (test) {
       '    buf.text("baz");',
       '  }})}); });',
       '}');
+   */
+});
+
+Tinytest.add("spacebars - Spacebars.index", function (test) {
+  test.equal(Spacebars.index(null, 'foo'), null);
+  test.equal(Spacebars.index('foo', 'foo'), undefined);
+  test.equal(Spacebars.index({x:1}, 'x'), 1);
+  test.equal(Spacebars.index(
+    {x:1, y: function () { return this.x+1; }}, 'y')(), 2);
+  test.equal(Spacebars.index(
+    function () {
+      return {x:1, y: function () { return this.x+1; }};
+    }, 'y')(), 2);
+
+  var m = 1;
+  var mget = function () {
+    return {
+      answer: m,
+      getAnswer: function () {
+        return this.answer;
+      }
+    };
+  };
+  var mgetDotAnswer = Spacebars.index(mget, 'answer');
+  test.equal(mgetDotAnswer(), 1);
+  m = 2;
+  test.equal(mgetDotAnswer(), 2);
+
+  m = 3;
+  var mgetDotGetAnswer = Spacebars.index(mget, 'getAnswer');
+  test.equal(mgetDotGetAnswer(), 3);
+  m = 4;
+  test.equal(mgetDotGetAnswer(), 4);
+
+  var closet = {
+    mget: mget,
+    mget2: function () {
+      return this.mget();
+    }
+  };
+
+  m = 5;
+  var f1 = Spacebars.index(closet, 'mget', 'answer');
+  test.equal(f1(), 5);
+  m = 6;
+  test.equal(f1(), 6);
+  var f2 = Spacebars.index(closet, 'mget2', 'answer');
+  m = 7;
+  test.equal(f2(), 7);
+  m = 8;
+  test.equal(f2(), 8);
+  var f3 = Spacebars.index(closet, 'mget2', 'getAnswer');
+  m = 9;
+  test.equal(f3(), 9);
+
+  test.equal(Spacebars.index(0, 'abc', 'def'), 0);
+  test.equal(Spacebars.index(function () { return null; }, 'abc', 'def')(), null);
+  test.equal(Spacebars.index(function () { return 0; }, 'abc', 'def')(), 0);
 });