From 23ef8f16ba0dc751d279cc179989851ec0ad00e1 Mon Sep 17 00:00:00 2001 From: Rafael Weinstein Date: Wed, 15 Jan 2014 14:31:54 -0800 Subject: [PATCH] add back delegate as second arg to createInstance() && clean up delegate/ref semantics, specifically: -assignment to bindingDelegate no longer causes instance production -assignment after instantiation has begun causes *future* instances to be affected. -the binding map is now stored on the template (and checked to make sure it maps the correct content) (as opposed to the content -- which could have been shared between templates with different delegates). R=arv BUG= Review URL: https://codereview.appspot.com/52880043 --- src/TemplateBinding.js | 79 +++++++++++++++++++---------- tests/tests.js | 112 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 160 insertions(+), 31 deletions(-) diff --git a/src/TemplateBinding.js b/src/TemplateBinding.js index 24471cd..848e252 100644 --- a/src/TemplateBinding.js +++ b/src/TemplateBinding.js @@ -395,9 +395,9 @@ if (!template.setModelFn_) { template.setModelFn_ = function() { template.setModelFnScheduled_ = false; - processBindings(template, - getBindings(template, template.prepareBindingFn_), - template.model_); + var map = getBindings(template, + template.delegate_ && template.delegate_.prepareBinding); + processBindings(template, map, template.model_); }; } @@ -432,14 +432,20 @@ return this.iterator_; }, - createInstance: function(model, instanceBindings) { + createInstance: function(model, bindingDelegate, delegate_, + instanceBindings_) { + if (bindingDelegate) + delegate_ = this.newDelegate_(bindingDelegate); + var content = this.ref.content; - var map = content.bindingMap_; - if (!map) { + var map = this.bindingMap_; + if (!map || map.content !== content) { // TODO(rafaelw): Setup a MutationObserver on content to detect // when the instanceMap is invalid. - map = createInstanceBindingMap(content, this.prepareBindingFn_) || []; - content.bindingMap_ = map; + map = createInstanceBindingMap(content, + delegate_ && delegate_.prepareBinding) || []; + map.content = content; + this.bindingMap_ = map; } var stagingDocument = getTemplateStagingDocument(this); @@ -457,8 +463,8 @@ var clone = cloneAndBindingInstance(child, instance, stagingDocument, map.children[i++], model, - this.bindingDelegate_, - instanceBindings); + delegate_, + instanceBindings_); clone.templateInstance_ = instanceRecord; } @@ -477,11 +483,21 @@ }, get bindingDelegate() { - return this.bindingDelegate_; + return this.delegate_.raw; + }, + + setDelegate_: function(delegate) { + this.delegate_ = delegate; + this.bindingMap_ = undefined; + if (this.iterator_) { + this.iterator_.instancePositionChangedFn_ = undefined; + this.iterator_.instanceModelFn_ = undefined; + } }, - setBindingDelegate_: function(bindingDelegate) { - this.bindingDelegate_ = bindingDelegate; + newDelegate_: function(bindingDelegate) { + if (!bindingDelegate) + return {}; function delegateFn(name) { var fn = bindingDelegate && bindingDelegate[name]; @@ -493,15 +509,20 @@ }; } - this.prepareBindingFn_ = delegateFn('prepareBinding'); - this.prepareInstanceModelFn_ = delegateFn('prepareInstanceModel'); - this.prepareInstancePositionChangedFn_ = - delegateFn('prepareInstancePositionChanged'); + return { + raw: bindingDelegate, + prepareBinding: delegateFn('prepareBinding'), + prepareInstanceModel: delegateFn('prepareInstanceModel'), + prepareInstancePositionChanged: + delegateFn('prepareInstancePositionChanged') + }; }, + // TODO(rafaelw): Assigning .bindingDelegate always succeeds. It may + // make sense to issue a warning or even throw if the template is already + // "activated", since this would be a strange thing to do. set bindingDelegate(bindingDelegate) { - this.setBindingDelegate_(bindingDelegate); - ensureSetModelScheduled(this); + this.setDelegate_(this.newDelegate_(bindingDelegate)); }, get ref() { @@ -744,9 +765,11 @@ return []; } - function cloneAndBindingInstance(node, parent, stagingDocument, bindings, model, - delegate, - instanceBindings, instanceRecord) { + function cloneAndBindingInstance(node, parent, stagingDocument, bindings, + model, + delegate, + instanceBindings, + instanceRecord) { var clone = parent.appendChild(stagingDocument.importNode(node, false)); var i = 0; @@ -761,7 +784,7 @@ if (bindings.isTemplate) { HTMLTemplateElement.decorate(clone, node); if (delegate) - clone.setBindingDelegate_(delegate); + clone.setDelegate_(delegate); } processBindings(clone, bindings, model, instanceBindings); @@ -967,14 +990,16 @@ ArrayObserver.applySplices(this.iteratedValue, this.presentValue, splices); + var delegate = template.delegate_; if (this.instanceModelFn_ === undefined) { this.instanceModelFn_ = - this.getDelegateFn(template.prepareInstanceModelFn_); + this.getDelegateFn(delegate && delegate.prepareInstanceModel); } if (this.instancePositionChangedFn_ === undefined) { this.instancePositionChangedFn_ = - this.getDelegateFn(template.prepareInstancePositionChangedFn_); + this.getDelegateFn(delegate && + delegate.prepareInstancePositionChanged); } var instanceCache = new Map; @@ -1005,8 +1030,8 @@ model = this.instanceModelFn_(model); if (model !== undefined) { - fragment = this.templateElement_.createInstance(model, - instanceBindings); + fragment = template.createInstance(model, undefined, delegate, + instanceBindings); } } diff --git a/tests/tests.js b/tests/tests.js index 571e87f..9298215 100644 --- a/tests/tests.js +++ b/tests/tests.js @@ -1409,6 +1409,31 @@ suite('Template Instantiation', function() { }); }); + test('Update Ref', function(done) { + var div = createTestHtml( + '' + + '' + + ''); + + var model = ['Fry']; + recursivelySetTemplateModel(div, model); + + then(function() { + assert.strictEqual(4, div.childNodes.length); + assert.strictEqual('Hi, Fry', div.childNodes[3].textContent); + + div.childNodes[2].setAttribute('ref', 'B'); + model.push('Leila'); + + }).then(function() { + assert.strictEqual(5, div.childNodes.length); + assert.strictEqual('Hi, Fry', div.childNodes[3].textContent); + assert.strictEqual('Hola, Leila', div.childNodes[4].textContent); + + done(); + }); + }); + test('BindWithDynamicRef', function(done) { var id = 't' + Math.round(100 * Math.random()); var div = createTestHtml( @@ -2560,8 +2585,7 @@ suite('Template Instantiation', function() { } }; - outer.bindingDelegate = delegate; - var instance = outer.createInstance(model); + var instance = outer.createInstance(model, delegate); assert.strictEqual(instance.firstChild.ref, outer.content.firstChild); assert.strictEqual('bar:replaced', instance.firstChild.nextSibling.textContent); @@ -2643,7 +2667,7 @@ suite('Template Instantiation', function() { items: [1] }; - template.bindingDelegate = { + var delegate = { prepareInstanceModel: function(template) { if (template.id == 'del') { return function(val) { @@ -2652,7 +2676,7 @@ suite('Template Instantiation', function() { } } }; - div.appendChild(template.createInstance(model)); + div.appendChild(template.createInstance(model, delegate)); then(function() { assert.equal('2', template.nextSibling.nextSibling.nextSibling.textContent); @@ -2941,6 +2965,86 @@ suite('Binding Delegate API', function() { }); }); + test('Update bindinDelegate with active template', function(done) { + function bindingHandler(prefix, path) { + return function(model) { + return new ObserverTransform(new PathObserver(model, path), + function(value) { + return prefix + ':' + value; + } + ); + } + } + + var delegateA = { + prepareBinding: function(path, name, node) { + if (path == '$ident') + return bindingHandler('a', 'id'); + + if (path == '$index') + return bindingHandler('i', 'index'); + }, + + prepareInstanceModel: function(template) { + return function(model) { + return { id: model }; + } + }, + + prepareInstancePositionChanged: function(template) { + return function(templateInstance, index) { + templateInstance.model.index = index; + } + } + }; + + var delegateB = { + prepareBinding: function(path, name, node) { + if (path == '$ident') + return bindingHandler('A', 'id'); + + if (path == '$index') + return bindingHandler('I', 'index'); + }, + + prepareInstanceModel: function(template) { + return function(model) { + return { id: model + '-narg' }; + } + }, + + prepareInstancePositionChanged: function(template) { + return function(templateInstance, index) { + templateInstance.model.index = 2 * index; + } + } + }; + + var model = [1, 2]; + + var div = createTestHtml( + ''); + var template = div.firstChild; + template.bindingDelegate = delegateA; + template.model = model; + + then(function() { + assert.strictEqual(3, div.childNodes.length); + assert.strictEqual('i:0 - a:1' , div.childNodes[1].textContent); + assert.strictEqual('i:1 - a:2' , div.childNodes[2].textContent); + + template.bindingDelegate = delegateB; + model.push(3); + }).then(function() { + assert.strictEqual(4, div.childNodes.length); + assert.strictEqual('i:0 - a:1' , div.childNodes[1].textContent); + assert.strictEqual('i:1 - a:2' , div.childNodes[2].textContent); + assert.strictEqual('I:4 - A:3-narg' , div.childNodes[3].textContent); + + done(); + }); + }); + test('Basic', function(done) { var model = { foo: 2, bar: 4 };