[mdv] Avoid observing placeholder arrays in TemplateIterator

pkg/mdv/lib/src/template_iterator.dart

Index: pkg/mdv/lib/src/template_iterator.dart
diff --git a/pkg/mdv/lib/src/template_iterator.dart b/pkg/mdv/lib/src/template_iterator.dart
index 82c4a7f24cb05a2877b4a32e036229eaeb016299..2a3a348407beadccfce47eaa16c9507937acdd18 100644
--- a/pkg/mdv/lib/src/template_iterator.dart
+++ b/pkg/mdv/lib/src/template_iterator.dart
@@ -179,57 +179,51 @@ class _BindingToken {
 class _TemplateIterator {
   final Element _templateElement;
   final List<Node> terminators = [];
-  final CompoundBinding inputs;
+  CompoundBinding inputs;
   List iteratedValue;
-  Object _lastValue;
+  bool closed = false;
 
   StreamSubscription _sub;
-  StreamSubscription _valueBinding;
 
-  _TemplateIterator(this._templateElement)
-      : inputs = new CompoundBinding(resolveInputs) {
-
-    _valueBinding = new PathObserver(inputs, 'value').bindSync(valueChanged);
+  _TemplateIterator(this._templateElement) {
+    inputs = new CompoundBinding(resolveInputs);

[justinfagnani, 2013/07/24 00:42:55]

why remove this from the initializer list, or make it non-final? I don't see an
assignment in the changes here.

[Jennifer Messerly, 2013/07/24 00:52:58]

it's sneaky: resolveInputs is no longer static, so we can't access it from an
initializer in Dart

   }
 
-  static Object resolveInputs(Map values) {
-    if (values.containsKey('if') && !_toBoolean(values['if'])) {
-      return null;
-    }
+  resolveInputs(Map values) {

[justinfagnani, 2013/07/24 00:42:55]

void?

[Jennifer Messerly, 2013/07/24 00:52:58]

Done.

[Jennifer Messerly, 2013/07/24 01:06:55]

update: it actually can't be void... CompoundBinding expects a value. I've
changed it to explicitly return null and added a comment

+    if (closed) return;
 
-    if (values.containsKey('repeat')) {
-      return values['repeat'];
-    }
-
-    if (values.containsKey('bind')) {
-      return [values['bind']];
+    if (values.containsKey('if') && !_toBoolean(values['if'])) {
+      valueChanged(null);
+    } else if (values.containsKey('repeat')) {
+      valueChanged(values['repeat']);
+    } else if (values.containsKey('bind')) {
+      valueChanged([values['bind']]);
+    } else {
+      valueChanged(null);
     }
-
-    return null;
   }
 
   void valueChanged(value) {
-    // TODO(jmesserly): should PathObserver do this for us?
-    var oldValue = _lastValue;
-    _lastValue = value;
-
-    if (value is! List) {
-      value = [];
-    }
+    if (value is! List) value = null;

[justinfagnani, 2013/07/24 00:42:55]

when do we want to use Iterable over List?

[Jennifer Messerly, 2013/07/24 00:52:58]

good question. I think we should follow up with MDV about this.

offhand, it definitely needs indexing for some of the operations, but we could
call .toList(). However I'm not sure if the typical Iterable is going to
implement observability (or how you would define it without indexes).

 
+    var oldValue = iteratedValue;
     unobserve();
     iteratedValue = value;
 
-    if (value is Observable) {
-      _sub = value.changes.listen(_handleChanges);
+    if (iteratedValue is Observable) {
+      _sub = iteratedValue.changes.listen(_handleChanges);
     }
 
-    int addedCount = iteratedValue.length;
-    var removedCount = oldValue is List ? (oldValue as List).length : 0;
-    if (addedCount == 0 && removedCount == 0) return; // nothing to do.
+    var splices = calculateSplices(
+        iteratedValue != null ? iteratedValue : [],
+        oldValue != null ? oldValue : []);
+
+    if (splices.length > 0) _handleChanges(splices);
 
-    _handleChanges([new ListChangeRecord(0, addedCount: addedCount,
-        removedCount: removedCount)]);
+    if (inputs.length == 0) {
+      close();
+      _mdv(_templateElement)._templateIterator = null;
+    }
   }
 
   Node getTerminatorAt(int index) {
@@ -295,13 +289,15 @@ class _TemplateIterator {
   }
 
   void _handleChanges(Iterable<ChangeRecord> splices) {
+    if (closed) return;
+
     splices = splices.where((s) => s is ListChangeRecord);
 
     var template = _templateElement;
     var syntax = template.attributes['syntax'];
 
     if (template.parentNode == null || template.document.window == null) {
-      abandon();
+      close();
       // TODO(jmesserly): MDV calls templateIteratorTable.delete(this) here,
       // but I think that's a no-op because only nodes are used as keys.
       // See https://github.com/Polymer/mdv/pull/114.
@@ -346,11 +342,13 @@ class _TemplateIterator {
     _sub = null;
   }
 
-  void abandon() {
+  void close() {
+    if (closed) return;
+
     unobserve();
-    _valueBinding.cancel();
     terminators.clear();
     inputs.dispose();
+    closed = true;
   }
 
   static void _unbindAllRecursively(Node node) {
@@ -360,7 +358,7 @@ class _TemplateIterator {
       // Make sure we stop observing when we remove an element.
       var templateIterator = nodeExt._templateIterator;
       if (templateIterator != null) {
-        templateIterator.abandon();
+        templateIterator.close();
         nodeExt._templateIterator = null;
       }
     }