Changes property options to support converter

CHANGELOG.md

@@ -9,16 +9,22 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
    PRs should document their user-visible changes (if any) in the
    Unreleased section, uncommenting the header as necessary.
 -->
+<!-- ### Added -->
+<!-- ### Changed -->
+<!-- ### Removed -->
+<!-- ### Fixed -->
 
 ## Unreleased
 
-### Fixed
-* [Breaking] User defined accessors are now wrapped to enable better composition ([#286](https://github.com/Polymer/lit-element/issues/286))
-
-<!-- ### Changed -->
 <!-- ### Added -->
+### Changed
+* [Breaking] Changes property options to add `converter`. This option works the same as the previous `type` option except that the `converter` methods now also get `type` as the second argument. This effectively changes `type` to be a hint for the `converter`. A default `converter` is used if none is provided and it now supports `Boolean`, `String`, `Number`, `Object`, and `Array` ([#264](https://github.com/Polymer/lit-element/issues/264)).
+* [Breaking] Numbers and strings now become null if their reflected attribute is removed (https://github.com/Polymer/lit-element/issues/264)).
+* [Breaking] Previously, when an attribute changed as a result of a reflecting property changing, the property was prevented from mutating again as can happen when a custom
+`converter` is used. Now, the oppose is also true. When a property changes as a result of an attribute changing, the attribute is prevented from mutating again (https://github.com/Polymer/lit-element/issues/264))
 <!-- ### Removed -->
 ### Fixed
+* [Breaking] User defined accessors are now wrapped to enable better composition ([#286](https://github.com/Polymer/lit-element/issues/286))
 * Type for `eventOptions` decorator now properly includes `passive` and `once` options ([#325](https://github.com/Polymer/lit-element/issues/325))
 
 ## [0.6.5] - 2018-12-13
@@ -83,8 +89,3 @@ https://github.com/Polymer/lit-html/pull/486).
 * The `firstUpdated` method should now always be called the first time the element
 updates, even if `shouldUpdate` initially returned `false`
 (https://github.com/Polymer/lit-element/pull/173).
-
-
-<!-- ### Changed -->
-<!-- ### Removed -->
-<!-- ### Fixed -->

README.md

@@ -35,19 +35,22 @@ for additional information on how to create templates for lit-element.
     If the value is `false`, the property is not added to the static `observedAttributes` getter.
     If `true` or absent, the lowercased property name is observed (e.g. `fooBar` becomes `foobar`).
     If a string, the string value is observed (e.g `attribute: 'foo-bar'`).
-    * `type`: Indicates how to serialize and deserialize the attribute to/from a property.
+    * `converter`: Indicates how to convert the attribute to/from a property.
     The value can be a function used for both serialization and deserialization, or it can
     be an object with individual functions via the optional keys, `fromAttribute` and `toAttribute`.
-    `type` defaults to the `String` constructor, and so does the `toAttribute` and `fromAttribute`
-    keys.
+    A default `converter` is used if none is provided; it supports
+    `Boolean`, `String`, `Number`, `Object`, and `Array`.
+    * `type`: Indicates the type of the property. This is used only as a hint for the
+    `converter` to determine how to convert the attribute
+    to/from a property. Note, when a property changes and the converter is used
+    to update the attribute, the property is never updated again as a result of
+    the attribute changing, and vice versa.
     * `reflect`: Indicates whether the property should reflect to its associated
-    attribute (as determined by the attribute option).
-    If `true`, when the property is set, the attribute which name is determined
-    according to the rules for the `attribute` property option, will be set to the
-    value of the property serialized using the rules from the `type` property option.
-    Note, `type: Boolean` has special handling by default which means that truthy
-    values result in the presence of the attribute, whereas falsy values result
-    in the absence of the attribute.
+    attribute (as determined by the attribute option). If `true`, when the
+    property is set, the attribute which name is determined according to the
+    rules for the `attribute` property option will be set to the value of the
+    property converted using the rules from the `type` and `converter`
+    property options.
     * `hasChanged`: A function that indicates whether a property should be considered
     changed when it is set and thus result in an update. The function should take the
     `newValue` and `oldValue` and return `true` if an update should be requested.

demo/lit-element.html

@@ -38,7 +38,7 @@
           foo: {},
           bar: {},
           whales: {type: Number},
-          fooBar: {type: {fromAttribute: parseInt, toAttribute: (value) => value + '-attr'}, reflect: true}
+          fooBar: {converter: {fromAttribute: parseInt, toAttribute: (value) => value + '-attr'}, reflect: true}
         }
       }
 

src/demo/ts-element.ts

@@ -4,7 +4,8 @@ class TSElement extends LitElement {
 
   @property() message = 'Hi';
 
-  @property({attribute : 'more-info', type: (value: string) => `[${value}]`})
+  @property(
+      {attribute : 'more-info', converter: (value: string) => `[${value}]`})
   extra = '';
 
   render() {

src/lib/updating-element.ts

@@ -33,27 +33,28 @@ const descriptorFromPrototype = (name: PropertyKey, proto: UpdatingElement) => {
 /**
  * Converts property values to and from attribute values.
  */
-export interface AttributeSerializer<T = any> {
+export interface ComplexAttributeConverter<Type = any, TypeHint = any> {
 
   /**
-   * Deserializing function called to convert an attribute value to a property
+   * Function called to convert an attribute value to a property
    * value.
    */
-  fromAttribute?(value: string): T;
+  fromAttribute?(value: string, type?: TypeHint): Type;
 
   /**
-   * Serializing function called to convert a property value to an attribute
+   * Function called to convert a property value to an attribute
    * value.
    */
-  toAttribute?(value: T): string|null;
+  toAttribute?(value: Type, type?: TypeHint): string|null;
 }
 
-type AttributeType<T = any> = AttributeSerializer<T>|((value: string) => T);
+type AttributeConverter<Type = any, TypeHint = any> =
+    ComplexAttributeConverter<Type>|((value: string, type?: TypeHint) => Type);
 
 /**
  * Defines options for a property accessor.
  */
-export interface PropertyDeclaration<T = any> {
+export interface PropertyDeclaration<Type = any, TypeHint = any> {
 
   /**
    * Indicates how and whether the property becomes an observed attribute.
@@ -65,23 +66,32 @@ export interface PropertyDeclaration<T = any> {
   attribute?: boolean|string;
 
   /**
-   * Indicates how to serialize and deserialize the attribute to/from a
-   * property. If this value is a function, it is used to deserialize the
-   * attribute value a the property value. If it's an object, it can have keys
-   * for `fromAttribute` and `toAttribute` where `fromAttribute` is the
-   * deserialize function and `toAttribute` is a serialize function used to set
-   * the property to an attribute. If no `toAttribute` function is provided and
+   * Indicates the type of the property. This is used only as a hint for the
+   * `converter` to determine how to convert the attribute
+   * to/from a property.
+   */
+  type?: TypeHint;
+
+  /**
+   * Indicates how to convert the attribute to/from a property. If this value
+   * is a function, it is used to convert the attribute value a the property
+   * value. If it's an object, it can have keys for `fromAttribute` and
+   * `toAttribute`. If no `toAttribute` function is provided and
    * `reflect` is set to `true`, the property value is set directly to the
-   * attribute.
+   * attribute. A default `converter` is used if none is provided; it supports
+   * `Boolean`, `String`, `Number`, `Object`, and `Array`. Note,
+   * when a property changes and the converter is used to update the attribute,
+   * the property is never updated again as a result of the attribute changing,
+   * and vice versa.
    */
-  type?: AttributeType<T>;
+  converter?: AttributeConverter<Type, TypeHint>;
 
   /**
    * Indicates if the property should reflect to an attribute.
    * If `true`, when the property is set, the attribute is set using the
    * attribute name determined according to the rules for the `attribute`
-   * property option and the value of the property serialized using the rules
-   * from the `type` property option.
+   * property option and the value of the property converted using the rules
+   * from the `converter` property option.
    */
   reflect?: boolean;
 
@@ -90,7 +100,7 @@ export interface PropertyDeclaration<T = any> {
    * it is set. The function should take the `newValue` and `oldValue` and
    * return `true` if an update should be requested.
    */
-  hasChanged?(value: T, oldValue: T): boolean;
+  hasChanged?(value: Type, oldValue: Type): boolean;
 
   /**
    * Indicates whether an accessor will be created for this property. By
@@ -118,9 +128,35 @@ type AttributeMap = Map<string, PropertyKey>;
 
 export type PropertyValues = Map<PropertyKey, unknown>;
 
-// serializer/deserializers for boolean attribute
-const fromBooleanAttribute = (value: string) => value !== null;
-const toBooleanAttribute = (value: string) => value ? '' : null;
+export const defaultConverter: ComplexAttributeConverter = {
+
+  toAttribute(value: any, type?: any) {
+    switch (type) {
+    case Boolean:
+      return value ? '' : null;
+    case Object:
+    case Array:
+      // if the value is `null` or `undefined` pass this through
+      // to allow removing/no change behavior.
+      return value == null ? value : JSON.stringify(value);
+    }
+    return value;
+  },
+
+  fromAttribute(value: any, type?: any) {
+    switch (type) {
+    case Boolean:
+      return value !== null;
+    case Number:
+      return value === null ? null : Number(value);
+    case Object:
+    case Array:
+      return JSON.parse(value);
+    }
+    return value;
+  }
+
+};
 
 export interface HasChanged {
   (value: unknown, old: unknown): boolean;
@@ -138,6 +174,7 @@ export const notEqual: HasChanged = (value: unknown, old: unknown): boolean => {
 const defaultPropertyDeclaration: PropertyDeclaration = {
   attribute : true,
   type : String,
+  converter : defaultConverter,
   reflect : false,
   hasChanged : notEqual
 };
@@ -146,9 +183,10 @@ const microtaskPromise = new Promise((resolve) => resolve(true));
 
 const STATE_HAS_UPDATED = 1;
 const STATE_UPDATE_REQUESTED = 1 << 2;
-const STATE_IS_REFLECTING = 1 << 3;
+const STATE_IS_REFLECTING_TO_ATTRIBUTE = 1 << 3;
+const STATE_IS_REFLECTING_TO_PROPERTY = 1 << 4;
 type UpdateState = typeof STATE_HAS_UPDATED|typeof STATE_UPDATE_REQUESTED|
-    typeof STATE_IS_REFLECTING;
+    typeof STATE_IS_REFLECTING_TO_ATTRIBUTE|typeof STATE_IS_REFLECTING_TO_PROPERTY;
 
 /**
  * Base element class which manages element properties and attributes. When
@@ -286,8 +324,8 @@ export abstract class UpdatingElement extends HTMLElement {
    * Returns the property name for the given attribute `name`.
    */
   private static _attributeNameForProperty(name: PropertyKey,
-                                           options?: PropertyDeclaration) {
-    const attribute = options !== undefined && options.attribute;
+                                           options: PropertyDeclaration) {
+    const attribute = options.attribute;
     return attribute === false
                ? undefined
                : (typeof attribute === 'string'
@@ -308,21 +346,16 @@ export abstract class UpdatingElement extends HTMLElement {
 
   /**
    * Returns the property value for the given attribute value.
-   * Called via the `attributeChangedCallback` and uses the property's `type`
-   * or `type.fromAttribute` property option.
+   * Called via the `attributeChangedCallback` and uses the property's
+   * `converter` or `converter.fromAttribute` property option.
    */
   private static _propertyValueFromAttribute(value: string,
-                                             options?: PropertyDeclaration) {
-    const type = options && options.type;
-    if (type === undefined) {
-      return value;
-    }
-    // Note: special case `Boolean` so users can use it as a `type`.
+                                             options: PropertyDeclaration) {
+    const type = options.type;
+    const converter = options.converter || defaultConverter;
     const fromAttribute =
-        type === Boolean
-            ? fromBooleanAttribute
-            : (typeof type === 'function' ? type : type.fromAttribute);
-    return fromAttribute ? fromAttribute(value) : value;
+        (typeof converter === 'function' ? converter : converter.fromAttribute);
+    return fromAttribute ? fromAttribute(value, type) : value;
   }
 
   /**
@@ -333,18 +366,16 @@ export abstract class UpdatingElement extends HTMLElement {
    * This uses the property's `reflect` and `type.toAttribute` property options.
    */
   private static _propertyValueToAttribute(value: unknown,
-                                           options?: PropertyDeclaration) {
-    if (options === undefined || options.reflect === undefined) {
+                                           options: PropertyDeclaration) {
+    if (options.reflect === undefined) {
       return;
     }
-    // Note: special case `Boolean` so users can use it as a `type`.
+    const type = options.type;
+    const converter = options.converter;
     const toAttribute =
-        options.type === Boolean
-            ? toBooleanAttribute
-            : (options.type &&
-                   (options.type as AttributeSerializer).toAttribute ||
-               String);
-    return toAttribute(value);
+        converter && (converter as ComplexAttributeConverter).toAttribute ||
+        defaultConverter.toAttribute;
+    return toAttribute!(value, type);
   }
 
   private _updateState: UpdateState = 0;
@@ -464,41 +495,49 @@ export abstract class UpdatingElement extends HTMLElement {
       name: PropertyKey, value: unknown,
       options: PropertyDeclaration = defaultPropertyDeclaration) {
     const ctor = (this.constructor as typeof UpdatingElement);
-    const attrValue = ctor._propertyValueToAttribute(value, options);
-    if (attrValue !== undefined) {
-      const attr = ctor._attributeNameForProperty(name, options);
-      if (attr !== undefined) {
-        // Track if the property is being reflected to avoid
-        // setting the property again via `attributeChangedCallback`. Note:
-        // 1. this takes advantage of the fact that the callback is synchronous.
-        // 2. will behave incorrectly if multiple attributes are in the reaction
-        // stack at time of calling. However, since we process attributes
-        // in `update` this should not be possible (or an extreme corner case
-        // that we'd like to discover).
-        // mark state reflecting
-        this._updateState = this._updateState | STATE_IS_REFLECTING;
-        if (attrValue === null) {
-          this.removeAttribute(attr);
-        } else {
-          this.setAttribute(attr, attrValue);
-        }
-        // mark state not reflecting
-        this._updateState = this._updateState & ~STATE_IS_REFLECTING;
+    const attr = ctor._attributeNameForProperty(name, options);
+    if (attr !== undefined) {
+      const attrValue = ctor._propertyValueToAttribute(value, options);
+      // an undefined value does not change the attribute.
+      if (attrValue === undefined) {
+        return;
       }
+      // Track if the property is being reflected to avoid
+      // setting the property again via `attributeChangedCallback`. Note:
+      // 1. this takes advantage of the fact that the callback is synchronous.
+      // 2. will behave incorrectly if multiple attributes are in the reaction
+      // stack at time of calling. However, since we process attributes
+      // in `update` this should not be possible (or an extreme corner case
+      // that we'd like to discover).
+      // mark state reflecting
+      this._updateState = this._updateState | STATE_IS_REFLECTING_TO_ATTRIBUTE;
+      if (attrValue == null) {
+        this.removeAttribute(attr);
+      } else {
+        this.setAttribute(attr, attrValue);
+      }
+      // mark state not reflecting
+      this._updateState = this._updateState & ~STATE_IS_REFLECTING_TO_ATTRIBUTE;
     }
   }
 
   private _attributeToProperty(name: string, value: string) {
     // Use tracking info to avoid deserializing attribute value if it was
     // just set from a property setter.
-    if (!(this._updateState & STATE_IS_REFLECTING)) {
-      const ctor = (this.constructor as typeof UpdatingElement);
-      const propName = ctor._attributeToPropertyMap.get(name);
-      if (propName !== undefined) {
-        const options = ctor._classProperties.get(propName);
-        this[propName as keyof this] =
-            ctor._propertyValueFromAttribute(value, options);
-      }
+    if (this._updateState & STATE_IS_REFLECTING_TO_ATTRIBUTE) {
+      return;
+    }
+    const ctor = (this.constructor as typeof UpdatingElement);
+    const propName = ctor._attributeToPropertyMap.get(name);
+    if (propName !== undefined) {
+      const options =
+          ctor._classProperties.get(propName) || defaultPropertyDeclaration;
+      // mark state reflecting
+      this._updateState = this._updateState | STATE_IS_REFLECTING_TO_PROPERTY;
+      this[propName as keyof this] =
+          ctor._propertyValueFromAttribute(value, options);
+      // mark state not reflecting
+      this._updateState = this._updateState & ~STATE_IS_REFLECTING_TO_PROPERTY;
     }
   }
 
@@ -526,8 +565,10 @@ export abstract class UpdatingElement extends HTMLElement {
       }
       // track old value when changing.
       this._changedProperties.set(name, oldValue);
-      // add to reflecting properties set
-      if (options.reflect === true) {
+      // Add to reflecting properties set if `reflect` is true and the property
+      // is not reflecting to the property from the attribute
+      if (options.reflect === true &&
+          !(this._updateState & STATE_IS_REFLECTING_TO_PROPERTY)) {
         if (this._reflectingProperties === undefined) {
           this._reflectingProperties = new Map();
         }