work around for https://github.com/Polymer/TemplateBinding/issues/150

pkg/template_binding/lib/src/instance_binding_map.dart

Index: pkg/template_binding/lib/src/instance_binding_map.dart
diff --git a/pkg/template_binding/lib/src/instance_binding_map.dart b/pkg/template_binding/lib/src/instance_binding_map.dart
index 9b1bf6d53221897e5ea75bcca8a76bfbdecaf946..71b0c12aac11962901cbbf56fbfb11be84eff1b7 100644
--- a/pkg/template_binding/lib/src/instance_binding_map.dart
+++ b/pkg/template_binding/lib/src/instance_binding_map.dart
@@ -6,30 +6,39 @@ part of template_binding;
 
 class _InstanceBindingMap {
   final List bindings;
-  final List<_InstanceBindingMap> children;
+  final Map<int, _InstanceBindingMap> children;

[Jennifer Messerly, 2013/12/09 23:49:22]

I noticed the JavaScript version uses a Map too. I didn't notice this change
during porting, but it's now `{}` in JavaScript instead of `[]`.

overall I think it's probably an okay change, it makes the Map more sparse,
though we could likely use an even more optimal data structure since typically
these maps are small and always have integer keys...

[Siggi Cherem (dart-lang), 2013/12/10 00:42:23]

It seemed to me that they still used a [] for the top-level "map", but {} for
the children, but this should work ok here.

[Jennifer Messerly, 2013/12/10 02:15:43]

yup. I like using a class for InstanceBindingMap though :). I find the untyped
JS code that uses heterogeneous arrays very hard to follow.

   final Node templateRef;
 
-  _InstanceBindingMap._(this.bindings, this.children, this.templateRef);
+  // Workaround for:
+  // https://github.com/Polymer/TemplateBinding/issues/150
+  final int numChildren;
 
-  factory _InstanceBindingMap(Node node, BindingDelegate delegate) {
-    var bindings = _getBindings(node, delegate);
-    Node templateRef = null;
+  _InstanceBindingMap._(this.bindings, this.children, this.templateRef,
+      this.numChildren);
+}
 
-    if (isSemanticTemplate(node)) templateRef = node;
+_InstanceBindingMap _createInstanceBindingMap(Node node,
+    BindingDelegate delegate) {
 
-    List children = null;
-    for (var c = node.firstChild, i = 0; c != null; c = c.nextNode, i++) {
-      var childMap = new _InstanceBindingMap(c, delegate);
-      if (childMap == null) continue;
+  var bindings = _getBindings(node, delegate);
+  Node templateRef = null;
 
-      if (children == null) children = new List(node.nodes.length);
-      children[i] = childMap;
-    }
+  if (isSemanticTemplate(node)) templateRef = node;
 
-    return new _InstanceBindingMap._(bindings, children, templateRef);
+  Map children = null;
+  int i = 0;
+  for (var c = node.firstChild; c != null; c = c.nextNode, i++) {
+    var childMap = _createInstanceBindingMap(c, delegate);
+    if (childMap == null) continue;
+
+    if (children == null) children = new HashMap();
+    children[i] = childMap;
   }
-}
 
+  if (bindings == null && children == null && templateRef == null) return null;

[Jennifer Messerly, 2013/12/09 23:49:22]

we were missing this important optimization.

+
+  return new _InstanceBindingMap._(bindings, children, templateRef, i);
+}
 
 void _addMapBindings(Node node, _InstanceBindingMap map, model,
     BindingDelegate delegate, List bound) {
@@ -48,8 +57,15 @@ void _addMapBindings(Node node, _InstanceBindingMap map, model,
 
   if (map.children == null) return;
 
-  int i = 0;
-  for (var c = node.firstChild; c != null; c = c.nextNode) {
-    _addMapBindings(c, map.children[i++], model, delegate, bound);
+  // To workaround https://github.com/Polymer/TemplateBinding/issues/150,

[Jennifer Messerly, 2013/12/09 23:49:22]

I'm not sure how robust this workaround is, but it seems safer than what the
code was doing.

[Siggi Cherem (dart-lang), 2013/12/10 00:42:23]

what would be more robust? a Map<Node, _InstanceBindingMap> with node instead of
int keys? 

[Jennifer Messerly, 2013/12/10 02:15:43]

the problem is, they aren't the same nodes. We're gather data from the
<template> and trying to apply it the instantiation.

Perhaps if the bindings were applied while the tree is being cloned, that would
be more robust. Then it wouldn't need to rely on child offset.

+  // we try and detect cases where creating a custom element resulted in extra
+  // children compared to what we expected. We assume these new children are all
+  // at the beginning, because _deepCloneIgnoreTemplateContent creates the
+  // element then appends the template content's children to the end.
+
+  int i = map.numChildren - node.nodes.length;
+  for (var c = node.firstChild; c != null; c = c.nextNode, i++) {
+    if (i < 0) continue;
+    _addMapBindings(c, map.children[i], model, delegate, bound);

[Siggi Cherem (dart-lang), 2013/12/10 00:42:23]

I'm not sure I follow this yet. Doesn't this 'i' need to be adjusted by some
offset here so that we match the original offset used by
createInstanceBindingMap?

For example,

int offset = map.numChildren - node.nodes.length;
int i = 0;
for (var c = node.firstChild; c != null; c = c.nextNode, i++) {
  if (i < offset) continue;
  _addMapBindings(c, map.children[i - offset], model, delegate, bound);

[Jennifer Messerly, 2013/12/10 02:15:43]

we're adjusting because it starts negative. Let's say map.numChildren is 3 (the
template had 3 children):
<template>
  <node><A><B><C>

 and node.nodes.length is 5 (the node's created must've added 2 children to
itself):

  <node><X><Y><A><B><C>

so we compute:

int i =  map.numChildren - node.nodes.length;
// so: i = 3 - 5; i = -2;

for (var c = node.firstChild; c != null; c = c.nextNode, i++) {

  // first iteration: i = -2, c = <X>
  if (i < 0) continue; // -2 < 0 so continue to next iteration

  // second iteration: i = -1, c = <Y>
  if (i < 0) continue; // -1 < 0 so continue to next iteration

  // third iteration: i = 0, c = <A>
  if (i < 0) continue; // 0 is not less than 0 so fall through

  _addMapBindings(<A>, map.children[0], model, delegate, bound);
  // ... etc ...

If it is more clear I could use two variables, I'm just folding "i" and "offset"
together. Your example code would work though, if the first line is changed to
be `int offset = node.nodes.length - map.numChildren;`

   }
 }