Refactor Spark to use observeChanges instead of observe

It uses an orderedDict instead of an array, and also this new version happens to
not do a ton of splices, which might help with performance in cases for really
large numbers of items.  Anyway, introduced a dependency on LocalCollection;
will break that soon.

packages/ordered-dict/ordered_dict.js

@@ -32,7 +32,7 @@
 
     empty: function () {
       var self = this;
-      return self._first !== null;
+      return !self._first;
     },
     putBefore: function (key, item, before) {
       var self = this;

packages/spark/spark.js

@@ -878,6 +878,22 @@ Spark.isolate = function (htmlFunc) {
 /* Lists                                                                      */
 /******************************************************************************/
 
+var idStringify;
+var idParse;
+
+if (typeof LocalCollection !== 'undefined') {
+  idStringify = function (id) {
+    if (id === null)
+      return id;
+    else
+      return LocalCollection._idStringify(id);
+  };
+  idParse = LocalCollection._idParse;
+} else {
+  idStringify = function (id) { return id; };
+  idParse = function (id) { return id; };
+}
+
 Spark.list = function (cursor, itemFunc, elseFunc) {
   elseFunc = elseFunc || function () { return ''; };
 
@@ -885,7 +901,7 @@ Spark.list = function (cursor, itemFunc, elseFunc) {
   // can change them later
   var callbacks = {};
   var observerCallbacks = {};
-  _.each(["added", "removed", "moved", "changed"], function (name) {
+  _.each(["addedBefore", "removed", "movedBefore", "changed"], function (name) {
     observerCallbacks[name] = function () {
       return callbacks[name].apply(null, arguments);
     };
@@ -895,13 +911,19 @@ Spark.list = function (cursor, itemFunc, elseFunc) {
   // XXX currently we count on observe() using only added() to deliver
   // the initial contents. are we allow to do that, or do we need to
   // implement removed/moved/changed here as well?
-  var initialContents = [];
+
+  var itemDict = new OrderedDict();
   _.extend(callbacks, {
-    added: function (item, beforeIndex) {
-      initialContents.splice(beforeIndex, 0, item);
+    addedBefore: function (id, item, before) {
+      var doc = EJSON.clone(item);
+      doc._id = id;
+      var elt = {doc: doc, liveRange: null};
+      itemDict.putBefore(idStringify(id),
+                         elt,
+                         idStringify(before));
     }
   });
-  var handle = cursor.observe(observerCallbacks);
+  var handle = cursor.observeChanges(observerCallbacks);
 
   // Get the renderer, if any
   var renderer = Spark._currentRenderer.get();
@@ -914,21 +936,18 @@ Spark.list = function (cursor, itemFunc, elseFunc) {
   // off for later.
   var html = '';
   var outerRange;
-  var itemRanges = [];
-  if (! initialContents.length)
+  if (itemDict.empty())
     html = elseFunc();
   else {
-    for (var i = 0; i < initialContents.length; i++) {
-      (function (i) {
-        html += maybeAnnotate(itemFunc(initialContents[i]),
-                              Spark._ANNOTATION_LIST_ITEM,
-                              function (range) {
-                                itemRanges[i] = range;
-                              });
-      })(i); // scope i to closure
-    }
+    itemDict.forEach(function (elt) {
+        html += maybeAnnotate(
+          itemFunc(elt.doc),
+          Spark._ANNOTATION_LIST_ITEM,
+          function (range) {
+            elt.liveRange = range;
+          });
+    });
   }
-  initialContents = null; // save memory
   var stopped = false;
   var cleanup = function () {
     handle.stop();
@@ -971,60 +990,65 @@ Spark.list = function (cursor, itemFunc, elseFunc) {
 
   // The DOM update callbacks.
   _.extend(callbacks, {
-    added: function (item, beforeIndex) {
+    addedBefore: function (id, fields, before) {
       later(function () {
-        var frag = Spark.render(_.bind(itemFunc, null, item));
+        var idStr = idStringify(id);
+        var befStr = idStringify(before);
+        var doc = EJSON.clone(fields);
+        doc._id = id;
+        var frag = Spark.render(_.bind(itemFunc, null, doc));
         DomUtils.wrapFragmentForContainer(frag, outerRange.containerNode());
         var range = makeRange(Spark._ANNOTATION_LIST_ITEM, frag);
 
-        if (! itemRanges.length) {
+        if (itemDict.empty()) {
           Spark.finalize(outerRange.replaceContents(frag));
-        } else if (beforeIndex === itemRanges.length) {
-          itemRanges[itemRanges.length - 1].insertAfter(frag);
+        } else if (before === null) {
+          itemDict.lastValue().liveRange.insertAfter(frag);
         } else {
-          itemRanges[beforeIndex].insertBefore(frag);
+          itemDict.get(befStr).liveRange.insertBefore(frag);
         }
-
-        itemRanges.splice(beforeIndex, 0, range);
+        itemDict.putBefore(idStr, {doc: doc, liveRange: range}, befStr);
       });
     },
 
-    removed: function (item, atIndex) {
+    removed: function (id) {
       later(function () {
-        if (itemRanges.length === 1) {
+        var idStr = idStringify(id);
+        if (itemDict.first() === itemDict.last()) {
           var frag = Spark.render(elseFunc);
           DomUtils.wrapFragmentForContainer(frag, outerRange.containerNode());
           Spark.finalize(outerRange.replaceContents(frag));
         } else
-          Spark.finalize(itemRanges[atIndex].extract());
+          Spark.finalize(itemDict.get(idStr).liveRange.extract());
 
-        itemRanges.splice(atIndex, 1);
+        itemDict.remove(idStr);
 
         notifyParentsRendered();
       });
     },
 
-    moved: function (item, oldIndex, newIndex) {
+    movedBefore: function (id, before) {
       later(function () {
-        if (oldIndex === newIndex)
-          return;
-
-        var frag = itemRanges[oldIndex].extract();
-        var range = itemRanges.splice(oldIndex, 1)[0];
-        if (newIndex === itemRanges.length)
-          itemRanges[itemRanges.length - 1].insertAfter(frag);
-        else
-          itemRanges[newIndex].insertBefore(frag);
-
-        itemRanges.splice(newIndex, 0, range);
-
+        var idStr = idStringify(id);
+        var befStr = idStringify(before);
+        var frag = itemDict.get(idStr).liveRange.extract();
+        if (before === null) {
+          itemDict.lastValue().liveRange.insertAfter(frag);
+        }
+        else {
+          itemDict.get(befStr).liveRange.insertBefore(frag);
+        }
+        itemDict.moveBefore(idStr, befStr);
         notifyParentsRendered();
       });
     },
 
-    changed: function (item, atIndex) {
+    changed: function (id, fields) {
       later(function () {
-        Spark.renderToRange(itemRanges[atIndex], _.bind(itemFunc, null, item));
+        var idStr = idStringify(id);
+        var elt = itemDict.get(idStr);
+        LocalCollection._applyChanges(elt.doc, fields);
+        Spark.renderToRange(elt.liveRange, _.bind(itemFunc, null, elt.doc));
       });
     }
   });

packages/spark/spark_tests.js

@@ -1093,7 +1093,7 @@ Tinytest.add("spark - list event handling", function(test) {
   // same thing, but with events wired by listChunk "added" and "removed"
   event_buf.length = 0;
   var lst = [];
-  lst.observe = function(callbacks) {
+  lst.observeChanges = function(callbacks) {
     lst.callbacks = callbacks;
     return {
       stop: function() {
@@ -1127,12 +1127,12 @@ Tinytest.add("spark - list event handling", function(test) {
   doClick();
   // add item
   lst.push({_id:'foo'});
-  lst.callbacks.added(lst[0], 0);
+  lst.callbacks.addedBefore(lst[0]._id, lst[0], null);
   Meteor.flush();
   test.equal(div.text().match(/\S+/)[0], 'foo');
   doClick();
   // remove item, back to "else" case
-  lst.callbacks.removed(lst[0], 0);
+  lst.callbacks.removed(lst[0]._id);
   lst.pop();
   Meteor.flush();
   test.equal(div.text().match(/\S+/)[0], 'else');
@@ -1952,9 +1952,9 @@ Tinytest.add("spark - list cursor stop", function(test) {
 
   var numHandles = 0;
   var observable = {
-    observe: function(x) {
-      x.added({_id:"123"}, 0);
-      x.added({_id:"456"}, 1);
+    observeChanges: function(x) {
+      x.addedBefore("123", {}, null);
+      x.addedBefore("456", {}, null);
       var handle;
       numHandles++;
       return handle = {
@@ -2162,13 +2162,13 @@ Tinytest.add("spark - list event data", function(test) {
   var div = OnscreenDiv(Meteor.render(function() {
     var html = Spark.list(
       {
-        observe: function(observer) {
-          observer.added({_id: '1', name: 'Foo'}, 0);
-          observer.added({_id: '2', name: 'Bar'}, 1);
+        observeChanges: function(observer) {
+          observer.addedBefore("1", {name: 'Foo'}, null);
+          observer.addedBefore("2", {name: 'Bar'}, null);
           // exercise callback path
           later = function() {
-            observer.added({_id: '3', name: 'Baz'}, 2);
-            observer.added({_id: '4', name: 'Qux'}, 3);
+            observer.addedBefore("3", {name: 'Baz'}, null);
+            observer.addedBefore("4", {name: 'Qux'}, null);
           };
           return { stop: function() {} };
         }
@@ -2327,7 +2327,7 @@ Tinytest.add("spark - cleanup", function(test) {
   var observeCount = 0;
   var stopCount = 0;
   var cursor = {
-    observe: function (callbacks) {
+    observeChanges: function (callbacks) {
       observeCount++;
       return {
         stop: function () {
@@ -3194,7 +3194,7 @@ Tinytest.add("spark - isolate inside landmark", function (test) {
 
 Tinytest.add("spark - nested onscreen processing", function (test) {
   var cursor = {
-    observe: function () { return { stop: function () {} }; }
+    observeChanges: function () { return { stop: function () {} }; }
   };
 
   var x = [];
@@ -3713,10 +3713,10 @@ Tinytest.add("spark - list update", function (test) {
 
   var lst = [];
   lst.callbacks = [];
-  lst.observe = function(callbacks) {
+  lst.observeChanges = function(callbacks) {
     lst.callbacks.push(callbacks);
-    _.each(lst, function(x, i) {
-      callbacks.added(x, i);
+    _.each(lst, function(x) {
+      callbacks.addedBefore(x._id, x, null);
     });
     return {
       stop: function() {
@@ -3728,7 +3728,7 @@ Tinytest.add("spark - list update", function (test) {
     var i = lst.length;
     lst.push({_id:'item'+i});
     _.each(lst.callbacks, function (callbacks) {
-      callbacks.added(lst[i], i);
+      callbacks.addedBefore(lst[i]._id, lst[i], null);
     });
   };
   var div = OnscreenDiv(Meteor.render(function() {

packages/templating/deftemplate.js

@@ -11,7 +11,7 @@
     Handlebars._default_helpers.each = function (arg, options) {
       // if arg isn't an observable (like LocalCollection.Cursor),
       // don't use this reactive implementation of #each.
-      if (!(arg && 'observe' in arg))
+      if (!(arg && 'observeChanges' in arg))
         return orig.call(this, arg, options);
 
       return Spark.list(

packages/templating/templating_tests.js

@@ -808,10 +808,10 @@ Tinytest.add("templating - #each rendered callback", function (test) {
   var cbks = [];
   var xs = ['a','b','c'];
   tmpl.helpers({entries: function() {
-    return { observe: function (callbacks) {
+    return { observeChanges: function (callbacks) {
       cbks.push(callbacks);
-      _.each(xs, function(x, i) {
-        callbacks.added({x:x}, i);
+      _.each(xs, function(x) {
+        callbacks.addedBefore(x, {x:x}, null);
       });
       return {
         stop: function () {
@@ -832,7 +832,7 @@ Tinytest.add("templating - #each rendered callback", function (test) {
   buf.length = 0;
 
   _.each(cbks, function (callbacks) {
-    callbacks.moved({x:'a'}, 0, 2);
+    callbacks.movedBefore('a', null);
   });
   test.equal(buf, []);
   Meteor.flush();
@@ -958,10 +958,10 @@ Tinytest.add("templating - unlabeled cursor", function (test) {
   var div = OnscreenDiv(Meteor.render(function () {
     R.get(); // create dependency
     return Template.test_unlabeled_cursor_a0(
-      {observe: function (callbacks) {
-        callbacks.added({}, 0);
-        callbacks.added({}, 1);
-        callbacks.added({}, 2);
+      {observeChanges: function (callbacks) {
+        callbacks.addedBefore('0', {}, null);
+        callbacks.addedBefore('1', {}, null);
+        callbacks.addedBefore('2', {}, null);
         return { stop: function () {} };
       }}
     );