Each/List refactoring in prep for reactive arrays

examples/unfinished/shark/client/shark.js

@@ -139,17 +139,23 @@ Meteor.startup(function () {
 });
 
 Meteor.startup(function () {
-  var c = function (n) { return UIComponent.create({render:function(buf) { buf(String(n)); }}); };
+  var c = UIComponent.extend({
+    render: function(buf) {
+      buf(String(this.data()));
+    }
+  });
 
-  L = _UI.List({elseContent: function () { return c('else'); }});
+  L = _UI.List({elseContent: function () {
+    return c(function () { return 'else'; });
+  }});
 
-  L.addItemBefore('a', c(1));
-  L.addItemBefore('b', c(2));
-  L.addItemBefore('c', c(3));
+  L.addItemBefore('a', c, 1);
+  L.addItemBefore('b', c, 2);
+  L.addItemBefore('c', c, 3);
   L.makeRoot();
   L.attach(document.body);
-  L.addItemBefore('d', c(4));
-  L.addItemBefore('e', c(5), 'b');
+  L.addItemBefore('d', c, 4);
+  L.addItemBefore('e', c, 5, 'b');
   L.moveItemBefore('d', 'c');
   L.moveItemBefore('a');
   L.removeItem('b');
@@ -157,8 +163,8 @@ Meteor.startup(function () {
   L.removeItem('c');
   L.removeItem('d');
   L.removeItem('e');
-  L.addItemBefore('a', c(1));
-  L.addItemBefore('b', c(2), 'a');
-  L.addItemBefore('c', c(3));
+  L.addItemBefore('a', c, 1);
+  L.addItemBefore('b', c, 2, 'a');
+  L.addItemBefore('c', c, 3);
   L.moveItemBefore('c', 'a');
 });

packages/ui/dom.js

@@ -287,9 +287,14 @@ Component.include({
     // We could be a root (and have no parent).  Parent could
     // theoretically be destroyed, or not yet built (if we
     // are currently building).
+    //
     // We use a falsy `parent.start` as a cue that this is a
     // rebuild, another case where we skip the start/end adjustment
     // logic.
+    //
+    // `attach` is special in that it is used during building
+    // and rebuilding; it is not required that the parent is
+    // completely built.
     if (parent && parent.stage === Component.BUILT &&
         parent.start) {
       if (parent.isEmpty()) {

packages/ui/each.js

@@ -7,17 +7,26 @@ _UI.List = Component.extend({
   constructed: function () {
     this._items = new OrderedDict;
   },
-  addItemBefore: function (id, comp, beforeId) {
-    this._items.putBefore(id, comp, beforeId);
+  addItemBefore: function (id, compType, data, beforeId) {
+    var self = this;
 
-    if (this.stage === Component.BUILT) {
-      if (this._else) {
-        this._else.remove();
-        this._else = null;
+    var comp = compType(function () {
+      this.dataDep.depend();
+      return this._data;
+    }, {
+      _data: data,
+      dataDep: new Deps.Dependency
+    });
+    self._items.putBefore(id, comp, beforeId);
+
+    if (self.stage === Component.BUILT) {
+      if (self._else) {
+        self._else.remove();
+        self._else = null;
       }
 
-      this.insertBefore(
-        comp, beforeId ? this._items.get(beforeId) : null);
+      self.insertBefore(
+        comp, beforeId ? self._items.get(beforeId) : null);
     }
   },
   removeItem: function (id) {
@@ -45,7 +54,24 @@ _UI.List = Component.extend({
   getItem: function (id) {
     return this._items.get(id) || null;
   },
+  setItemData: function (id, newData) {
+    var comp = this.getItem(id);
+    if (! comp)
+      throw new Error("No such item: " + id);
+    // Do a `===` check even though it's weak
+    if (newData !== comp._data) {
+      comp._data = newData;
+      comp.dataDep.changed();
+    }
+  },
   render: function (buf) {
+    // This component reactively rebuilds when any dependencies
+    // here are invalidated.
+    //
+    // The "item" methods cannot be called from here; they assume
+    // they are not operating during the build, but either
+    // before or after it.
+
     var self = this;
     if (self._items.empty()) {
       buf(self._else = self.elseContent());
@@ -66,49 +92,54 @@ _UI.List = Component.extend({
 _UI.Each = Component.extend({
   typeName: 'Each',
   List: _UI.List,
+  _oldData: null,
+  init: function () {
+    var self = this;
+    self._list = self.List({
+      elseContent: function (/**/) {
+        return self.elseContent.apply(self, arguments);
+      }
+    });
+    // add outside of the rebuild cycle
+    self.add(self._list);
+  },
   render: function (buf) {
     var self = this;
+    var list = self._list;
 
     // XXX support arrays too.
     // XXX and objects.
     // For now, we assume the data is a database cursor.
-    var cursor = self.data();
-    if (! cursor)
-      return;
+    var newData = self.data();
+    // Do a `===` check even though it's weak
+    if (newData !== self._oldData) {
+      self._oldData = newData;
 
-    var list = new self.List({
-      elseContent: self.elseContent
-    });
+      if (newData && newData.observe) {
+        var cursor = newData;
 
-    cursor.observe({
-      _no_indices: true,
-      addedAt: function (doc, i, beforeId) {
-        var comp = self.content(function () {
-          this.dataDep.depend();
-          return this._data;
-        }, {
-          _data: doc,
-          dataDep: new Deps.Dependency
+        cursor.observe({
+          _no_indices: true,
+          addedAt: function (doc, i, beforeId) {
+            list.addItemBefore(Meteor.idStringify(doc._id),
+                               self.content, doc,
+                               beforeId && Meteor.idStringify(beforeId));
+          },
+          removed: function (doc) {
+            list.removeItem(Meteor.idStringify(doc._id));
+          },
+          movedTo: function (doc, i, j, beforeId) {
+            list.moveItemBefore(Meteor.idStringify(doc._id),
+                                beforeId && Meteor.idStringify(beforeId));
+          },
+          changed: function (newDoc) {
+            list.setItemData(Meteor.idStringify(newDoc._id), newDoc);
+          }
         });
-        // XXX could `before` be a falsy ID?  Technically
-        // idStringify seems to allow for them -- though
-        // OrderedDict won't call stringify on a falsy arg.
-        list.addItemBefore(Meteor.idStringify(doc._id), comp,
-                           beforeId && Meteor.idStringify(beforeId));
-      },
-      removed: function (doc) {
-        list.removeItem(Meteor.idStringify(doc._id));
-      },
-      movedTo: function (doc, i, j, beforeId) {
-        list.moveItemBefore(Meteor.idStringify(doc._id),
-                            beforeId && Meteor.idStringify(beforeId));
-      },
-      changed: function (newDoc) {
-        var comp = list.getItem(Meteor.idStringify(newDoc._id));
-        comp._data = newDoc;
-        comp.dataDep.changed();
       }
-    });
+    }
+    // XXX we fail on switching to empty; should use
+    // patching replace for that.
 
     buf(list);
   }

packages/ui/render.js

@@ -91,7 +91,6 @@ makeRenderBuffer = function (component, options) {
       randomString = randomString || Random.id();
       var commentString = randomString + '_' + (commentUid++);
       push('<!--', commentString, '-->');
-      component.add(arg);
       componentsToAttach = componentsToAttach || {};
       componentsToAttach[commentString] = arg;
     } else if (arg.type) {
@@ -192,6 +191,12 @@ makeRenderBuffer = function (component, options) {
                 if (n === root.lastChild)
                   end = comp;
               }
+              if (comp.stage === Component.INITIAL) {
+                component.add(comp);
+              } else if (comp.parent !== component) {
+                throw new Error("Component used in render must be a child " +
+                                "(or addable as one)");
+              }
               comp.attach(parent, n);
               parent.removeChild(n);
               delete componentsToAttach[n.nodeValue];