[element-mixin] Do not create property accessors unless a property ef…

…fect 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).

lib/mixins/element-mixin.html

@@ -231,11 +231,8 @@
       if (info.observer) {
         proto._createPropertyObserver(name, info.observer, allProps[info.observer]);
       }
-      // always ensure an accessor is made for properties but don't stomp
-      // on existing values.
-      if (!info.readOnly && !(name in proto)) {
-        proto._createPropertyAccessor(name);
-      }
+      // always create the mapping from attribute back to property for deserialization.
+      proto._addPropertyToAttributeMap(name);
     }
 
     /**

lib/mixins/properties-changed.html

@@ -105,17 +105,30 @@
          * @protected
          */
         _createPropertyAccessor(property, readOnly) {
+          this._addPropertyToAttributeMap(property);
           if (!this.hasOwnProperty('__dataHasAccessor')) {
             this.__dataHasAccessor = Object.assign({}, this.__dataHasAccessor);
           }
+          if (!this.__dataHasAccessor[property]) {
+            this.__dataHasAccessor[property] = true;
+            this._definePropertyAccessor(property, readOnly);
+          }
+        }
+
+        /**
+         * Adds the given `property` to a map matching attribute names
+         * to property names, using `attributeNameForProperty`. This map is
+         * used when deserializing attribute values to properties.
+         *
+         * @param {string} property Name of the property
+         */
+        _addPropertyToAttributeMap(property) {
           if (!this.hasOwnProperty('__dataAttributes')) {
             this.__dataAttributes = Object.assign({}, this.__dataAttributes);
           }
-          if (!this.__dataHasAccessor[property]) {
-            this.__dataHasAccessor[property] = true;
+          if (!this.__dataAttributes[property]) {
             const attr = this.constructor.attributeNameForProperty(property);
             this.__dataAttributes[attr] = property;
-            this._definePropertyAccessor(property, readOnly);
           }
         }
 

test/unit/polymer.element.html

@@ -389,23 +389,6 @@ <h1>Sub template</h1>
       assert.equal(el.computedProp, true);
     });
 
-    test('properties have accessors', function() {
-      assert.isTrue(el.__dataHasAccessor.accessor);
-      el._propertiesChanged = sinon.spy();
-      el.accessor = 'hi';
-      assert.equal(el._propertiesChanged.args[0][0].accessor, 'hi');
-    });
-
-    test('properties with no effects do not stomp existing accessors', function() {
-      assert.notOk(el.__dataHasAccessor.noStomp);
-      el._propertiesChanged = sinon.spy();
-      el.accessor = 'hi';
-      el.noStomp = 'hi';
-      const props = el._propertiesChanged.args[0][0];
-      assert.equal(props.accessor, 'hi');
-      assert.notOk(props.noStomp);
-    });
-
     test('attributes', function() {
       const fixtureEl = fixture('my-element-attr');
       assert.equal(fixtureEl.prop, 'attr');