From a5cd67dc9c0112233ff7c18c09d3b5ccbb449462 Mon Sep 17 00:00:00 2001
From: David Greenspan <dgreenspan@alum.mit.edu>
Date: Thu, 25 Jul 2013 00:55:41 -0700
Subject: [PATCH] #each ported (untested)

---
 packages/ui2/each.js    | 178 +++++++++++++++++++++-------------------
 packages/ui2/package.js |   3 +-
 packages/ui2/render.js  |  18 ++--
 3 files changed, 110 insertions(+), 89 deletions(-)

diff --git a/packages/ui2/each.js b/packages/ui2/each.js
index e436b6c60c..9c214078b3 100644
--- a/packages/ui2/each.js
+++ b/packages/ui2/each.js
@@ -1,25 +1,33 @@
+var UI = UI2;
+var Component = UI.Component;
+
+
+// XXXX COMBINE LIST AND EACH???
 
 // `id` arguments to this class MUST be non-empty strings
 UI.List = Component.extend({
   typeName: 'List',
   _items: null, // OrderedDict of id -> Component
   _else: null, // Component
-  constructed: function () {
+  init: function () {
     this._items = new OrderedDict;
   },
-  addItemBefore: function (id, compType, data, beforeId) {
+  addItemBefore: function (id, comp, data, beforeId) {
     var self = this;
 
-    var comp = compType(function () {
-      this.dataDep.depend();
-      return this._data;
+    var dep = new Deps.Dependency;
+    comp = comp.withData(_extend(function () {
+      dep.depend();
+      return data;
     }, {
-      _data: data,
-      dataDep: new Deps.Dependency
-    });
+      $set: function (v) {
+        data = v;
+        dep.changed();
+      }}));
+
     self._items.putBefore(id, comp, beforeId);
 
-    if (self.stage === Component.BUILT) {
+    if (self.isBuilt) {
       if (self._else) {
         self._else.remove();
         self._else = null;
@@ -32,12 +40,22 @@ UI.List = Component.extend({
   removeItem: function (id) {
     var comp = this._items.remove(id);
 
-    if (this.stage === Component.BUILT) {
+    if (this.isBuilt) {
       comp.remove();
       if (this._items.empty()) {
-        this._else = this.elseContent();
-        if (this._else)
-          this.append(this._else);
+        // XXX figure out what to do about the need for
+        // `constructify` in places like this.  We want
+        // `content` and `elseContent` to be able to be
+        // functions so that templates can include
+        // children with reactive types.  However,
+        // while render buffers are programmed to accept
+        // the union type of {uninited component |
+        // inited component | function}, the built-time
+        // component methods don't.  Maybe they should.
+        //
+        // XXX this doesn't reactively update elseContent
+        this._else = constructify(this.elseContent);
+        this.append(this._else);
       }
     }
   },
@@ -45,7 +63,7 @@ UI.List = Component.extend({
     var comp = this._items.get(id);
     this._items.moveBefore(id, beforeId);
 
-    if (this.stage === Component.BUILT) {
+    if (this.isBuilt) {
       comp.detach();
       this.insertBefore(
         comp, beforeId ? this._items.get(beforeId) : null);
@@ -58,11 +76,8 @@ UI.List = Component.extend({
     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();
-    }
+
+    comp.data.$set(newData);
   },
   numItems: function () {
     return this._items.size();
@@ -77,10 +92,11 @@ UI.List = Component.extend({
 
     var self = this;
     if (self._items.empty()) {
-      buf(self._else = self.elseContent());
+      self._else = buf.write(self.elseContent);
     } else {
+      self._else = null;
       self._items.forEach(function (comp) {
-        buf(comp);
+        buf.write(comp);
       });
     }
   },
@@ -110,7 +126,7 @@ UI.List = Component.extend({
     var rand = Random.id().slice(0,4);
     return {
       // here only, id may be null
-      add: function (id, compType, data) {
+      add: function (id, comp, data) {
         var origId = id;
         while ((! id) || seenIds.hasOwnProperty(id))
           id = (origId || '') + rand + (counter++);
@@ -132,15 +148,15 @@ UI.List = Component.extend({
         // least-common-subsequence would, be we do reuse existing
         // components and move them to the right place.
         if (ptr === id) {
-          // XXX we don't deal the case where compType is different
-          // from the original compType.
+          // XXX we don't deal the case where comp is different
+          // from the original comp.  Oops.
           self.setItemData(id, data);
           ptr = items.next(ptr);
         } else if (items.has(id)) {
           self.moveItemBefore(id, ptr);
           self.setItemData(id, data);
         } else {
-          self.addItemBefore(id, compType, data, ptr);
+          self.addItemBefore(id, comp, data, ptr);
         }
       },
       end: function () {
@@ -158,13 +174,13 @@ 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);
-      }
+      // doesn't bind `this` if `elseContent` is a function,
+      // but then `elseContent` is not a real method, right?
+      // just a function you call for reactivity purposes?
+      elseContent: self.elseContent
     });
     // add outside of the rebuild cycle
     self.add(self._list);
@@ -190,67 +206,63 @@ UI.Each = Component.extend({
     var self = this;
     var list = self._list;
 
-    var newData = self.data();
-    // Do a `===` check even though it's weak
-    // XXX no, don't, because of the case where we
-    // are given the same array but it has been
-    // mutated, like test-in-browser does.
-    // But if we did some "is it the same" check
-    // it would go here.
-    if (true || newData !== self._oldData) {
-      self._oldData = newData;
+    var data = self.get();
 
-      var replacer = list.beginReplace();
+    // if `content` reactively changes type, we simply rebuild
+    // completely.
+    var content = (typeof self.content === 'function' ?
+                   self.content() : self.content);
 
-      if (! newData) {
-        // nothing to do
-      } else if (typeof newData.length === 'number' &&
-                 typeof newData.splice === 'function') {
-        // looks like an array
-        var array = newData;
+    var replacer = list.beginReplace();
 
-        for (var i=0, N=array.length; i<N; i++) {
-          var x = array[i];
-          replacer.add(self._getId(x), self.content, x);
-        }
+    if (! data) {
+      // no items to enumerate in the replacement
+    } else if (typeof data.length === 'number' &&
+               typeof data.splice === 'function') {
+      // looks like an array
+      var array = data;
 
-      } else if (newData.observe) {
-        var cursor = newData;
-
-        // we assume that `observe` will only call `addedAt` (and
-        // not other callbacks) before returning (which is specced),
-        // and further that these calls will be in document order
-        // (which isn't).
-        cursor.observe({
-          _no_indices: true,
-          addedAt: function (doc, i, beforeId) {
-            var id = Meteor.idStringify(doc._id);
-            if (replacer)
-              replacer.add(id, self.content, doc);
-            else
-              list.addItemBefore(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);
-          }
-        });
-      } else {
-        for (var k in newData)
-          replacer.add(k, self.content, newData[k]);
+      for (var i=0, N=array.length; i<N; i++) {
+        var x = array[i];
+        replacer.add(self._getId(x), self.content, x);
       }
 
-      replacer.end();
-      replacer = null; // so cursor.observe callbacks stop using it
+    } else if (data.observe) {
+      var cursor = data;
+
+      // we assume that `observe` will only call `addedAt` (and
+      // not other callbacks) before returning (which is specced),
+      // and further that these calls will be in document order
+      // (which isn't).
+      cursor.observe({
+        _no_indices: true,
+        addedAt: function (doc, i, beforeId) {
+          var id = Meteor.idStringify(doc._id);
+          if (replacer)
+            replacer.add(id, content, doc);
+          else
+            list.addItemBefore(id, 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);
+        }
+      });
+    } else {
+      for (var k in data)
+        replacer.add(k, content, data[k]);
     }
 
-    buf(list);
+    replacer.end();
+    replacer = null; // so cursor.observe callbacks stop using it
+
+    buf.write(list);
   }
 });
diff --git a/packages/ui2/package.js b/packages/ui2/package.js
index 57af12f042..984ef879a1 100644
--- a/packages/ui2/package.js
+++ b/packages/ui2/package.js
@@ -12,7 +12,8 @@ Package.on_use(function (api) {
                  'attrs.js',
                  'render.js',
                  'fields.js',
-                 'components.js']);
+                 'components.js',
+                 'each.js']);
 });
 
 Package.on_test(function (api) {
diff --git a/packages/ui2/render.js b/packages/ui2/render.js
index 8fa58f9118..bd09ad8f64 100644
--- a/packages/ui2/render.js
+++ b/packages/ui2/render.js
@@ -31,7 +31,7 @@ var GT_OR_QUOTE = /[>'"]/;
 // in the special case where `comp` is an already-inited
 // component, just return it.  In this latter case, the
 // `props` argument must be falsy.
-var constructify = function (comp, props) {
+constructify = function (comp, props) {
   if (props)
     // comp had better be uninited! (or will throw)
     return comp.extend(props);
@@ -100,11 +100,12 @@ makeRenderBuffer = function (component, options) {
     push('<!--', commentString, '-->');
     componentsToAttach = componentsToAttach || {};
     componentsToAttach[commentString] = comp;
+    return comp;
   };
 
   var handle = function (arg) {
     if (arg == null) {
-      return;
+      // nothing to do
     } else if (typeof arg === 'string') {
       // "HTML"
       push(arg);
@@ -113,7 +114,7 @@ makeRenderBuffer = function (component, options) {
       if (! arg.isInited)
         arg = arg.extend();
 
-      handleComponent(arg);
+      return handleComponent(arg);
     } else if ((typeof arg === 'function') || arg.child) {
       // `componentFunction`, or
       // `{child: componentOrFunction, props: object}`
@@ -180,7 +181,12 @@ makeRenderBuffer = function (component, options) {
       }
       // the autorun above closes down over this var:
       var curChild = constructify(curComp, props);
-      handleComponent(curChild);
+      // return something the caller of `buf.write` can't get
+      // any other way if `arg` involved a componentFunction:
+      // the actual component created.  If the arg we are
+      // handling is the last arg to `buf.write`, it will return
+      // this value.
+      return handleComponent(curChild);
     } else if (arg.attrs) {
       // `{attrs: functionOrDictionary }`
       // attrs object inserts zero or more `name="value"` items
@@ -223,8 +229,10 @@ makeRenderBuffer = function (component, options) {
 
   var buf = {};
   buf.write = function (/*args*/) {
+    var ret;
     for (var i = 0; i < arguments.length; i++)
-      handle(arguments[i]);
+      ret = handle(arguments[i]);
+    return ret;
   };
 
   buf.getHtml = function () {