[ProperitesChanged] Fix deserialization (#4996)

* [ProperitesChanged] Fix deserialization

A change merged with the `basic-element` caused the `type` argument to `_deserializeValue` to be called as a function to determine the deserialized output. This was an unintended breaking change from master and is reverted here. For example, the `basic-element` change regressed using a class constructor as a property type.

* Remove cruft

* Update docs

* Test verbose wct output on firefox

* Avoid Chrome and FF running in parallel on Travis to avoid timeouts.

.travis.yml

@@ -13,7 +13,8 @@ before_script:
 - bower install
 - gulp lint-eslint
 script:
-- xvfb-run wct
+- xvfb-run wct -l chrome
+- xvfb-run wct -l firefox
 - if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then wct -s 'windows 10/microsoftedge@14' -s 'windows 10/microsoftedge@15' -s 'windows 8.1/internet explorer@11' -s 'os x 10.11/safari@9' -s 'macos 10.12/safari@10' -s 'macos 10.12/safari@11' -s 'Linux/chrome@41'; fi
 env:
   global:

lib/mixins/properties-changed.html

@@ -25,17 +25,14 @@
      * or more property accessors (getter/setter pair) that enqueue an async
      * (batched) `_propertiesChanged` callback.
      *
-     * For basic usage of this mixin, simply declare attributes to observe via
-     * the standard `static get observedAttributes()`, implement `_propertiesChanged`
-     * on the class, and then call `MyClass.createPropertiesForAttributes()` once
-     * on the class to generate property accessors for each observed attribute
-     * prior to instancing. Last, call `this._enableProperties()` in the element's
+     * For basic usage of this mixin, call `MyClass.createProperties(props)`
+     * once at class definition time to create property accessors for properties
+     * named in props, implement `_propertiesChanged` to react as desired to
+     * property changes, and implement `static get observedAttributes()` and
+     * include lowercase versions of any property names that should be set from
+     * attributes. Last, call `this._enableProperties()` in the element's
      * `connectedCallback` to enable the accessors.
      *
-     * Any `observedAttributes` will automatically be
-     * deserialized via `attributeChangedCallback` and set to the associated
-     * property using `dash-case`-to-`camelCase` convention.
-     *
      * @mixinFunction
      * @polymer
      * @memberof Polymer
@@ -348,9 +345,8 @@
          * considered as a change and cause the `_propertiesChanged` callback
          * to be enqueued.
          *
-         * The default implementation returns `true` for primitive types if a
-         * strict equality check fails, and returns `true` for all Object/Arrays.
-         * The method always returns false for `NaN`.
+         * The default implementation returns `true` if a strict equality
+         * check fails. The method always returns false for `NaN`.
          *
          * Override this method to e.g. provide stricter checking for
          * Objects/Arrays when using immutable patterns.
@@ -454,9 +450,9 @@
         /**
          * Converts a typed JavaScript value to a string.
          *
-         * This method is called by Polymer when setting JS property values to
-         * HTML attributes.  Users may override this method on Polymer element
-         * prototypes to provide serialization for custom types.
+         * This method is called when setting JS property values to
+         * HTML attributes.  Users may override this method to provide
+         * serialization for custom types.
          *
          * @param {*} value Property value to serialize.
          * @return {string | undefined} String serialized from the provided
@@ -476,9 +472,8 @@
          *
          * This method is called when reading HTML attribute values to
          * JS properties.  Users may override this method to provide
-         * deserialization for custom `type`s. The given `type` is executed
-         * as a function with the value as an argument. The `Boolean` `type`
-         * is specially handled such that an empty string returns true.
+         * deserialization for custom `type`s. Types for `Boolean`, `String`,
+         * and `Number` convert attributes to the expected types.
          *
          * @param {?string} value Value to deserialize.
          * @param {*=} type Type to deserialize the string to.
@@ -488,10 +483,10 @@
           switch (type) {
             case Boolean:
               return (value !== null);
-            case String:
-              return value;
+            case Number:
+              return Number(value);
             default:
-              return typeof type == 'function' ? type(value) : value;
+              return value;
           }
         }
 

test/unit/polymer.properties-mixin.html

@@ -20,14 +20,21 @@
     <script>
     HTMLImports.whenReady(function() {
 
+      class Glar {}
+      class Blar {}
+
       class MyElement extends Polymer.PropertiesMixin(HTMLElement) {
 
         static get properties() {
           return {
             prop: String,
             noStomp: String,
             id: String,
-            camelCase: String
+            camelCase: String,
+            boo: Boolean,
+            num: Number,
+            glar: Glar,
+            blar: Blar
           };
         }
 
@@ -40,6 +47,13 @@
           this.noStomp = 'stomped';
         }
 
+        _deserializeValue(value, type) {
+          if (type == Glar) {
+            return 'glar';
+          }
+          return super._deserializeValue(value, type);
+        }
+
         get noStomp() {
           return 'noStomp';
         }
@@ -279,6 +293,29 @@
       assert.equal(fixtureEl.prop, null);
     });
 
+    test('deserialize standard types', function() {
+      el.setAttribute('boo', '');
+      assert.equal(el.boo, true);
+      el.removeAttribute('boo');
+      assert.equal(el.boo, false);
+      el.setAttribute('prop', '1');
+      assert.equal(el.prop, '1');
+      el.setAttribute('num', '1');
+      assert.equal(el.num, 1);
+      el.setAttribute('num', '0');
+      assert.equal(el.num, 0);
+    });
+
+    test('deserialize class constructor type', function() {
+      el.setAttribute('blar', 'a');
+      assert.equal(el.blar, 'a');
+    });
+
+    test('deserialize custom type via `_deserializeValue`', function() {
+      el.setAttribute('glar', 'b');
+      assert.equal(el.glar, 'glar');
+    });
+
     test('reflecting attributes', function() {
       const fixtureEl = fixture('my-element-attr');
       fixtureEl.prop = 'propValue';