Skip to content

Commit

Permalink
Replace _compoundInitializationEffect with statically-initialized lit…
Browse files Browse the repository at this point in the history
…erals in the template for attributes & textContent, and by configuring literal values of properties in _configureAnnotationReferences.
  • Loading branch information
kevinpschaaf committed Oct 13, 2015
1 parent a300862 commit 2f1bd31
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 55 deletions.
17 changes: 14 additions & 3 deletions src/lib/annotations/annotations.html
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,12 @@
_parseTextNodeAnnotation: function(node, list) {
var parts = this._parseBindings(node.textContent);
if (parts) {
// NOTE: use a space here so the textNode remains; some browsers
// (IE) evacipate an empty textNode.
node.textContent = ' ';
// Initialize the textContent with any literal parts
// NOTE: default to a space here so the textNode remains; some browsers
// (IE) evacipate an empty textNode following cloneNode/importNode.
node.textContent = parts.map(function(part) {
return part.literal;
}).join('') || ' ';
var annote = {
bindings: [{
kind: 'text',
Expand Down Expand Up @@ -291,6 +294,13 @@
name = name.slice(0, -1);
kind = 'attribute';
}
// Initialize attribute bindings with any literal parts
var literal = parts.map(function(part) {
return part.literal;
}).join('');
if (kind == 'attribute') {
node.setAttribute(name, literal);
}
// Clear attribute before removing, since IE won't allow removing
// `value` attribute if it previously had a value (can't
// unconditionally set '' before removing since attributes with `$`
Expand All @@ -311,6 +321,7 @@
kind: kind,
name: name,
parts: parts,
literal: literal,
isCompound: parts.length !== 1
};
}
Expand Down
4 changes: 0 additions & 4 deletions src/lib/bind/effects.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@
!effect.parts[0].negate;
},

_compoundInitializationEffect: function(source, value, effect) {
this._applyEffectValue(effect);
},

_annotationEffect: function(source, value, effect) {
if (source != effect.value) {
value = this._get(effect.value);
Expand Down
63 changes: 36 additions & 27 deletions src/standard/annotations.html
Original file line number Diff line number Diff line change
Expand Up @@ -219,42 +219,51 @@
},

// push configuration references at configure time
_configureAnnotationReferences: function() {
this._configureTemplateContent();
this._configureCompoundBindings();
},

// nested template contents have been stored prototypically to avoid
// unnecessary duplication, here we put references to the
// indirected contents onto the nested template instances
_configureTemplateContent: function() {
this._notes.forEach(function(note, i) {
_configureAnnotationReferences: function(config) {

This comment has been minimized.

Copy link
@sorvell

sorvell Oct 14, 2015

Contributor

Can we factor this into smaller functions?

var notes = this._notes;
var nodes = this._nodes;
for (var i=0; i<notes.length; i++) {
var note = notes[i];
var node = nodes[i];
// nested template contents have been stored prototypically to avoid
// unnecessary duplication, here we put references to the
// indirected contents onto the nested template instances
if (note.templateContent) {
// note: we can rely on _nodes being set here and having the same
// index as _notes
this._nodes[i]._content = note.templateContent;
}
}, this);
},

// Compound bindings utilize private storage on the node to store
// the current state of each value that will be concatenated to generate
// the final property/attribute/text value
// Here we initialize the private storage array on the node with any
// literal parts that won't change (could get fancy and use WeakMap)
_configureCompoundBindings: function() {
this._notes.forEach(function(note, i) {
var node = this._nodes[i];
note.bindings.forEach(function(binding) {
// Compound bindings utilize private storage on the node to store
// the current state of each value that will be concatenated to generate
// the final property/attribute/text value
// Here we initialize the private storage array on the node with any
// literal parts that won't change (could get fancy and use WeakMap),
// and configure property bindings to children with the literal parts
// (textContent and annotations were already initialized in the template)
var bindings = note.bindings;
for (var j=0; j<bindings.length; j++) {
var binding = bindings[j];
if (binding.isCompound) {
var storage = node.__compoundStorage__ ||
(node.__compoundStorage__ = {});
storage[binding.name] = binding.parts.map(function(part) {
return part.literal;
});
var parts = binding.parts;
var literals = new Array(binding.parts);
for (var k=0; k<parts.length; k++) {
literals[k] = parts[k].literal;
}
var name = binding.name;
storage[name] = literals;
if (binding.kind == 'property') {
var literal = literals.join('');
if (node._configValue) {
node._configValue(name, literal);
} else {
node[name] = literal;
}
}
}
});
}, this);
}
}
},

// construct `$` map (from id annotations)
Expand Down
8 changes: 0 additions & 8 deletions src/standard/effectBuilder.html
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,6 @@
});
}
}
// We need to ensure compound bindings with literals push the literals
// through regardless of whether annotation dependencies initialized
// this is done with a special compoundInitialization effect which simply
// kicks _applyEffectValue to push the literals through
if (note.isCompound) {
note.index = index;
this._addPropertyEffect('__static__', 'compoundInitialization', note);
}
},

_addAnnotatedComputationEffect: function(note, part, index) {
Expand Down
26 changes: 24 additions & 2 deletions test/unit/bind-elements.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
computed-from-tricky-literals2='{{computeFromTrickyLiterals(3,"tricky\,&#39;zot&#39;" )}}'
computed-from-no-args="{{computeFromNoArgs( )}}"
no-computed="{{foobared(noInlineComputed)}}"
compound1="{{cpnd1}}{{ cpnd2 }}{{cpnd3.prop}}{{ computeCompound(cpnd4, cpnd5, 'literal')}}"
compound2="literal1 {{cpnd1}} literal2 {{cpnd2}}{{cpnd3.prop}} literal3 {{computeCompound(cpnd4, cpnd5, 'literal')}} literal4"
compoundAttr1$="{{cpnd1}}{{ cpnd2 }}{{cpnd3.prop}}{{ computeCompound(cpnd4, cpnd5, 'literal')}}"
compoundAttr2$="literal1 {{cpnd1}} literal2 {{cpnd2}}{{cpnd3.prop}} literal3 {{computeCompound(cpnd4, cpnd5, 'literal')}} literal4"
>
Test
</div>
<x-prop id="boundProps"
prop1="{{cpnd1}}{{ cpnd2 }}{{cpnd3.prop}}{{ computeCompound(cpnd4, cpnd5, 'literal')}}"
prop2="literal1 {{cpnd1}} literal2 {{cpnd2}}{{cpnd3.prop}} literal3 {{computeCompound(cpnd4, cpnd5, 'literal')}} literal4"
></x-prop>
<span id="boundText">{{text}}</span>
<span idtest id="{{boundId}}"></span>
<s id="computedContent">{{computeFromTrickyLiterals(3, 'tricky\,\'zot\'')}}</s>
Expand Down Expand Up @@ -364,6 +366,26 @@
});
</script>

<script>
Polymer({
is: 'x-prop',
properties: {
prop1: {
value: 'default',
observer: 'prop1Changed'
},
prop2: {
value: 'default',
observer: 'prop2Changed'
}
},
created: function() {
this.prop1Changed = sinon.spy();
this.prop2Changed = sinon.spy();
}
});
</script>

<script>
Polymer({
is: 'x-notifies1',
Expand Down
35 changes: 24 additions & 11 deletions test/unit/bind.html
Original file line number Diff line number Diff line change
Expand Up @@ -703,36 +703,38 @@

test('compound adjacent property bindings', function() {
var el = document.createElement('x-basic');
assert.equal(el.$.boundChild.compound1, '');
assert.equal(el.$.boundProps.prop1, '');
assert.isTrue(el.$.boundProps.prop1Changed.calledOnce);
el.cpnd2 = 'cpnd2';
assert.equal(el.$.boundChild.compound1, 'cpnd2');
assert.equal(el.$.boundProps.prop1, 'cpnd2');
el.cpnd1 = 'cpnd1';
el.cpnd3 = {prop: 'cpnd3'};
assert.equal(el.$.boundChild.compound1, 'cpnd1cpnd2cpnd3');
assert.equal(el.$.boundProps.prop1, 'cpnd1cpnd2cpnd3');
el.cpnd4 = 'cpnd4';
assert.equal(el.$.boundChild.compound1, 'cpnd1cpnd2cpnd3');
assert.equal(el.$.boundProps.prop1, 'cpnd1cpnd2cpnd3');
el.cpnd5 = 'cpnd5';
assert.equal(el.$.boundChild.compound1, 'cpnd1cpnd2cpnd3literalcpnd5cpnd4');
assert.equal(el.$.boundProps.prop1, 'cpnd1cpnd2cpnd3literalcpnd5cpnd4');
});

test('compound property bindings with literals', function() {
var el = document.createElement('x-basic');
assert.equal(el.$.boundChild.compound2, 'literal1 literal2 literal3 literal4');
assert.equal(el.$.boundProps.prop2, 'literal1 literal2 literal3 literal4');
assert.isTrue(el.$.boundProps.prop2Changed.calledOnce);
el.cpnd1 = 'cpnd1';
el.cpnd2 = 'cpnd2';
el.cpnd3 = {prop: 'cpnd3'};
el.cpnd4 = 'cpnd4';
el.cpnd5 = 'cpnd5';
assert.equal(el.$.boundChild.compound2, 'literal1 cpnd1 literal2 cpnd2cpnd3 literal3 literalcpnd5cpnd4 literal4');
assert.equal(el.$.boundProps.prop2, 'literal1 cpnd1 literal2 cpnd2cpnd3 literal3 literalcpnd5cpnd4 literal4');
el.cpnd1 = null;
el.cpnd2 = undefined;
el.cpnd3 = {};
el.cpnd4 = '';
el.cpnd5 = '';
assert.equal(el.$.boundChild.compound2, 'literal1 literal2 literal3 literal literal4');
assert.equal(el.$.boundProps.prop2, 'literal1 literal2 literal3 literal literal4');
});

test('compound adjacent property bindings', function() {
test('compound adjacent attribute bindings', function() {
var el = document.createElement('x-basic');
assert.equal(el.$.boundChild.getAttribute('compoundAttr1'), '');
el.cpnd2 = 'cpnd2';
Expand All @@ -746,7 +748,7 @@
assert.equal(el.$.boundChild.getAttribute('compoundAttr1'), 'cpnd1cpnd2cpnd3literalcpnd5cpnd4');
});

test('compound property bindings with literals', function() {
test('compound property attribute with literals', function() {
var el = document.createElement('x-basic');
assert.equal(el.$.boundChild.getAttribute('compoundAttr2'), 'literal1 literal2 literal3 literal4');
el.cpnd1 = 'cpnd1';
Expand All @@ -765,7 +767,10 @@

test('compound adjacent textNode bindings', function() {
var el = document.createElement('x-basic');
assert.equal(el.$.compound1.textContent, '');
// The single space is due to the gambit to prevent empty text nodes
// from being evacipated on IE during importNode from the template; it will
// only be there the when in the virgin state after cloning the template
assert.equal(el.$.compound1.textContent, ' ');
el.cpnd2 = 'cpnd2';
assert.equal(el.$.compound1.textContent, 'cpnd2');
el.cpnd1 = 'cpnd1';
Expand All @@ -775,6 +780,14 @@
assert.equal(el.$.compound1.textContent, 'cpnd1cpnd2cpnd3');
el.cpnd5 = 'cpnd5';
assert.equal(el.$.compound1.textContent, 'cpnd1cpnd2cpnd3literalcpnd5cpnd4');
// Once the binding evaluates back to '', it will in fact be ''
el.computeCompound = function() { return ''; };
el.cpnd1 = null;
el.cpnd2 = '';
el.cpnd3 = {prop: null};
el.cpnd4 = null;
el.cpnd5 = '';
assert.equal(el.$.compound1.textContent, '');
});

test('compound textNode bindings with literals', function() {
Expand Down

0 comments on commit 2f1bd31

Please sign in to comment.