Skip to content

Commit 4177d9c

Browse files
author
Steven Orvell
committed
[element-mixin] Do not create property accessors unless a property effect exists
Reverts an unintentional breaking change. Previously (2.3.x) property accessors were created only for properties in the properties object which had explicit property effects. In 2.4.0, property accessors were created for any property name returned in the properties object. This was a breaking change in rare cases (for example in https://github.com/PolymerElements/iron-a11y-keys-behavior). This change reverts the 2.4.0 behavior. Accessors are now again only added for properties which have explicit effects. In addition, `_addPropetyToAttributeMap` was added to `PropertiesChanged` to record a mapping from the attribute to the property name (needed for deserialization).
1 parent 9941451 commit 4177d9c

File tree

3 files changed

+18
-25
lines changed

3 files changed

+18
-25
lines changed

lib/mixins/element-mixin.html

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,8 @@
231231
if (info.observer) {
232232
proto._createPropertyObserver(name, info.observer, allProps[info.observer]);
233233
}
234-
// always ensure an accessor is made for properties but don't stomp
235-
// on existing values.
236-
if (!info.readOnly && !(name in proto)) {
237-
proto._createPropertyAccessor(name);
238-
}
234+
// always create the mapping from attribute back to property for deserialization.
235+
proto._addPropertyToAttributeMap(name);
239236
}
240237

241238
/**

lib/mixins/properties-changed.html

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,30 @@
105105
* @protected
106106
*/
107107
_createPropertyAccessor(property, readOnly) {
108+
this._addPropertyToAttributeMap(property);
108109
if (!this.hasOwnProperty('__dataHasAccessor')) {
109110
this.__dataHasAccessor = Object.assign({}, this.__dataHasAccessor);
110111
}
112+
if (!this.__dataHasAccessor[property]) {
113+
this.__dataHasAccessor[property] = true;
114+
this._definePropertyAccessor(property, readOnly);
115+
}
116+
}
117+
118+
/**
119+
* Adds the given `property` to a map matching attribute names
120+
* to property names, using `attributeNameForProperty`. This map is
121+
* used when deserializing attribute values to properties.
122+
*
123+
* @param {string} property Name of the property
124+
*/
125+
_addPropertyToAttributeMap(property) {
111126
if (!this.hasOwnProperty('__dataAttributes')) {
112127
this.__dataAttributes = Object.assign({}, this.__dataAttributes);
113128
}
114-
if (!this.__dataHasAccessor[property]) {
115-
this.__dataHasAccessor[property] = true;
129+
if (!this.__dataAttributes[property]) {
116130
const attr = this.constructor.attributeNameForProperty(property);
117131
this.__dataAttributes[attr] = property;
118-
this._definePropertyAccessor(property, readOnly);
119132
}
120133
}
121134

test/unit/polymer.element.html

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -389,23 +389,6 @@ <h1>Sub template</h1>
389389
assert.equal(el.computedProp, true);
390390
});
391391

392-
test('properties have accessors', function() {
393-
assert.isTrue(el.__dataHasAccessor.accessor);
394-
el._propertiesChanged = sinon.spy();
395-
el.accessor = 'hi';
396-
assert.equal(el._propertiesChanged.args[0][0].accessor, 'hi');
397-
});
398-
399-
test('properties with no effects do not stomp existing accessors', function() {
400-
assert.notOk(el.__dataHasAccessor.noStomp);
401-
el._propertiesChanged = sinon.spy();
402-
el.accessor = 'hi';
403-
el.noStomp = 'hi';
404-
const props = el._propertiesChanged.args[0][0];
405-
assert.equal(props.accessor, 'hi');
406-
assert.notOk(props.noStomp);
407-
});
408-
409392
test('attributes', function() {
410393
const fixtureEl = fixture('my-element-attr');
411394
assert.equal(fixtureEl.prop, 'attr');

0 commit comments

Comments
 (0)