From 421acd2c8197e5d949be36113dc46319637913a2 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 13 Dec 2018 10:07:50 -0800 Subject: [PATCH 01/13] Changes property options to support `converter` Fixes #264 Changes `type` to be only a hint to the `converter` option which has the previous `type` functionality, an object with `toAttribute` and `fromAttribute` or just a function which is `fromAttribute`. In addition to the `value` these functions now also get the property's `type`. Also provides a default converter that supports `Boolean`, `String`, `Number`, `Object`, and `Array` out of the box. In addition, numbers and strings now become `null` if their reflected attribute is removed. --- CHANGELOG.md | 5 +- README.md | 7 +- demo/lit-element.html | 2 +- src/demo/ts-element.ts | 2 +- src/lib/updating-element.ts | 116 +++++++++++-------- src/test/lit-element_test.ts | 219 ++++++++++++++++++++++++++++++++--- 6 files changed, 278 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c04b3a98..ae7c513c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased * Types for the `property` and `customElement` decorators updated ([#288](https://github.com/Polymer/lit-element/issues/288) and [#291](https://github.com/Polymer/lit-element/issues/291)). - +### Changed +* 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`. In addition, numbers and strings now become null if their reflected attribute is removed. ([#264](https://github.com/Polymer/lit-element/issues/264)). + + diff --git a/README.md b/README.md index 7a7b850c..070dd701 100644 --- a/README.md +++ b/README.md @@ -35,11 +35,12 @@ 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 serialize and deserialize 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. + * `type`: Indicates the type of the property. This is used only as a hint for the + `converter` to determine how to serialize and deserialize the attribute + to/from a property. * `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 diff --git a/demo/lit-element.html b/demo/lit-element.html index 00659f49..e0764a4f 100644 --- a/demo/lit-element.html +++ b/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} } } diff --git a/src/demo/ts-element.ts b/src/demo/ts-element.ts index 4df4c2c5..b17e965e 100644 --- a/src/demo/ts-element.ts +++ b/src/demo/ts-element.ts @@ -4,7 +4,7 @@ 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() { diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 047fbfd0..e7afbc2e 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -15,22 +15,22 @@ /** * Converts property values to and from attribute values. */ -export interface AttributeSerializer { +export interface ComplexAttributeConverter { /** * Deserializing function called to convert an attribute value to a property * value. */ - fromAttribute?(value: string): T; + fromAttribute?(value: string, type?: any): T; /** * Serializing function called to convert a property value to an attribute * value. */ - toAttribute?(value: T): string|null; + toAttribute?(value: T, type?: any): string|null; } -type AttributeType = AttributeSerializer|((value: string) => T); +type AttributeConverter = ComplexAttributeConverter|((value: string, type?: any) => T); /** * Defines options for a property accessor. @@ -46,6 +46,13 @@ export interface PropertyDeclaration { */ attribute?: boolean|string; + /** + * Indicates the type of the property. This is used only as a hint for the + * converter to determine how to serialize and deserialize the attribute + * to/from a property. + */ + type?: T; + /** * Indicates how to serialize and deserialize the attribute to/from a * property. If this value is a function, it is used to deserialize the @@ -56,14 +63,14 @@ export interface PropertyDeclaration { * `reflect` is set to `true`, the property value is set directly to the * attribute. */ - type?: AttributeType; + converter?: AttributeConverter; /** * 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. + * from the `converter` property option. */ reflect?: boolean; @@ -90,9 +97,33 @@ type AttributeMap = Map; export type PropertyValues = Map; -// 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: + return 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; @@ -110,6 +141,7 @@ export const notEqual: HasChanged = (value: unknown, old: unknown): boolean => { const defaultPropertyDeclaration: PropertyDeclaration = { attribute : true, type : String, + converter: defaultConverter, reflect : false, hasChanged : notEqual }; @@ -260,21 +292,15 @@ 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`. - const fromAttribute = - type === Boolean - ? fromBooleanAttribute - : (typeof type === 'function' ? type : type.fromAttribute); - return fromAttribute ? fromAttribute(value) : value; + const converter = options && options.converter || defaultConverter; + const fromAttribute = (typeof converter === 'function' ? converter : converter.fromAttribute); + return fromAttribute ? fromAttribute(value, type) : value; } /** @@ -289,14 +315,10 @@ export abstract class UpdatingElement extends HTMLElement { if (options === undefined || options.reflect === undefined) { return; } - // Note: special case `Boolean` so users can use it as a `type`. - const toAttribute = - options.type === Boolean - ? toBooleanAttribute - : (options.type && - (options.type as AttributeSerializer).toAttribute || - String); - return toAttribute(value); + const type = options && options.type; + const converter = options && options.converter; + const toAttribute = converter && (converter as ComplexAttributeConverter).toAttribute || defaultConverter.toAttribute; + return toAttribute!(value, type); } private _updateState: UpdateState = 0; @@ -416,27 +438,25 @@ 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); + // 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; } } diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index b96c6884..12725f6a 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -161,12 +161,12 @@ suite('LitElement', () => { atTr : {attribute : true}, customAttr : {attribute : 'custom', reflect : true}, hasChanged : {hasChanged}, - fromAttribute : {type : fromAttribute}, - toAttribute : {reflect : true, type : {toAttribute}}, + fromAttribute : {converter : fromAttribute}, + toAttribute : {reflect : true, converter : {toAttribute}}, all : { attribute : 'all-attr', hasChanged, - type : {fromAttribute, toAttribute}, + converter : {fromAttribute, toAttribute}, reflect : true }, }; @@ -246,6 +246,179 @@ suite('LitElement', () => { assert.equal(el.updateCount, 6); }); + test('property option `converter` can use `type` info', async() => { + const FooType = {name: 'FooType'}; + const converter = { + fromAttribute: (value: any, type: any) => { + return `fromAttribute: ${String(type.name)}`; + }, + toAttribute: (value: any, type: any) => { + return `toAttribute: ${String(type.name)}`; + } + }; + + class E extends LitElement { + static get properties() { + return { + num: {type: Number, converter, reflect: true}, + str: {type: String, converter, reflect: true}, + foo: {type: FooType, converter, reflect: true} + }; + } + + num?: any; + str?: any; + foo?: any; + + render() { return html``; } + } + customElements.define(generateElementName(), E); + const el = new E(); + container.appendChild(el); + await el.updateComplete; + el.num = 5; + el.str = 'hi'; + el.foo = 'zoink'; + await el.updateComplete; + assert.equal(el.getAttribute('num'), 'toAttribute: Number'); + assert.equal(el.getAttribute('str'), 'toAttribute: String'); + assert.equal(el.getAttribute('foo'), 'toAttribute: FooType'); + el.removeAttribute('num'); + el.removeAttribute('str'); + el.removeAttribute('foo'); + await el.updateComplete; + assert.equal(el.num, 'fromAttribute: Number'); + assert.equal(el.str, 'fromAttribute: String'); + assert.equal(el.foo, 'fromAttribute: FooType'); + assert.equal(el.getAttribute('num'), 'toAttribute: Number'); + assert.equal(el.getAttribute('str'), 'toAttribute: String'); + assert.equal(el.getAttribute('foo'), 'toAttribute: FooType'); + }); + + test('property/attribute values when attributes removed', async () => { + class E extends LitElement { + static get properties() { + return { + bool: {type: Boolean}, + num: {type: Number}, + str: {type: String}, + obj: {type: Object}, + arr: {type: Array}, + reflectBool: {type: Boolean, reflect: true}, + reflectNum: {type: Number, reflect: true}, + reflectStr: {type: String, reflect: true}, + reflectObj: {type: Object, reflect: true}, + reflectArr: {type: Array, reflect: true}, + defaultBool: {type: Boolean}, + defaultNum: {type: Number}, + defaultStr: {type: String}, + defaultObj: {type: Object}, + defaultArr: {type: Array}, + defaultReflectBool: {type: Boolean, reflect: true}, + defaultReflectNum: {type: Number, reflect: true}, + defaultReflectStr: {type: String, reflect: true}, + defaultReflectObj: {type: Object, reflect: true}, + defaultReflectArr: {type: Array, reflect: true}, + }; + } + + bool?: any; + num?: any; + str?: any; + obj?: any; + arr?: any; + reflectBool?: any; + reflectNum?: any; + reflectStr?: any; + reflectObj?: any; + reflectArr?: any; + defaultBool = false; + defaultNum = 0; + defaultStr = ''; + defaultObj = {defaultObj: false}; + defaultArr = [1]; + defaultReflectBool = false; + defaultReflectNum = 0; + defaultReflectStr = 'defaultReflectStr'; + defaultReflectObj = {defaultReflectObj: true}; + defaultReflectArr = [1, 2]; + + + render() { return html``; } + } + const name = generateElementName(); + customElements.define(name, E); + container.innerHTML = `<${name} bool num="2" str="str" obj='{"obj": true}' + arr='[1]' reflectBool reflectNum="3" reflectStr="reflectStr" + reflectObj ='{"reflectObj": true}' reflectArr="[1, 2]" + defaultBool defaultNum="4" defaultStr="defaultStr" + defaultObj='{"defaultObj": true}' defaultArr="[1, 2, 3]"> + `; + const el = container.firstChild as E; + await el.updateComplete; + assert.equal(el.bool, true); + assert.equal(el.num, 2); + assert.equal(el.str, 'str'); + assert.deepEqual(el.obj, {obj: true}); + assert.deepEqual(el.arr, [1]); + assert.equal(el.reflectBool, true); + assert.equal(el.reflectNum, 3); + assert.equal(el.reflectStr, 'reflectStr'); + assert.deepEqual(el.reflectObj, {reflectObj: true}); + assert.deepEqual(el.reflectArr, [1, 2]); + assert.equal(el.defaultBool, true); + assert.equal(el.defaultNum, 4); + assert.equal(el.defaultStr, 'defaultStr'); + assert.deepEqual(el.defaultObj, {defaultObj: true}); + assert.deepEqual(el.defaultArr, [1, 2, 3]); + assert.equal(el.defaultReflectBool, false); + assert.equal(el.defaultReflectNum, 0); + assert.equal(el.defaultReflectStr, 'defaultReflectStr'); + assert.deepEqual(el.defaultReflectObj, {defaultReflectObj: true}); + assert.deepEqual(el.defaultReflectArr, [1, 2]); + el.removeAttribute('bool'); + el.removeAttribute('num'); + el.removeAttribute('str'); + el.removeAttribute('obj'); + el.removeAttribute('arr'); + el.removeAttribute('reflectBool'); + el.removeAttribute('reflectNum'); + el.removeAttribute('reflectStr'); + el.removeAttribute('reflectObj'); + el.removeAttribute('reflectArr'); + el.removeAttribute('defaultBool'); + el.removeAttribute('defaultNum'); + el.removeAttribute('defaultStr'); + el.removeAttribute('defaultObj'); + el.removeAttribute('defaultArr'); + el.removeAttribute('defaultReflectBool'); + el.removeAttribute('defaultReflectNum'); + el.removeAttribute('defaultReflectStr'); + el.removeAttribute('defaultReflectObj'); + el.removeAttribute('defaultReflectArr'); + await el.updateComplete; + assert.equal(el.bool, false); + assert.equal(el.num, null); + assert.equal(el.str, null); + assert.deepEqual(el.obj, null); + assert.deepEqual(el.arr, null); + assert.equal(el.reflectBool, false); + assert.equal(el.reflectNum, null); + assert.equal(el.reflectStr, null); + assert.deepEqual(el.reflectObj, null); + assert.deepEqual(el.reflectArr, null); + assert.equal(el.defaultBool, false); + assert.equal(el.defaultNum, null); + assert.equal(el.defaultStr, null); + assert.deepEqual(el.defaultObj, null); + assert.deepEqual(el.defaultArr, null); + assert.equal(el.defaultReflectBool, false); + assert.equal(el.defaultReflectNum, null); + assert.equal(el.defaultReflectStr, null); + assert.deepEqual(el.defaultReflectObj, null); + assert.deepEqual(el.defaultReflectArr, null); + }); + test('property options via decorator', async () => { const hasChanged = (value: any, old: any) => old === undefined || value > old; @@ -258,12 +431,12 @@ suite('LitElement', () => { @property({attribute : 'custom', reflect: true}) customAttr = 'customAttr'; @property({hasChanged}) hasChanged = 10; - @property({type : fromAttribute}) fromAttribute = 1; - @property({reflect : true, type: {toAttribute}}) toAttribute = 1; + @property({converter : fromAttribute}) fromAttribute = 1; + @property({reflect : true, converter: {toAttribute}}) toAttribute = 1; @property({ attribute : 'all-attr', hasChanged, - type: {fromAttribute, toAttribute}, + converter: {fromAttribute, toAttribute}, reflect: true }) all = 10; @@ -342,12 +515,12 @@ suite('LitElement', () => { class E extends LitElement { @property({hasChanged}) hasChanged = 10; - @property({type : fromAttribute}) fromAttribute = 1; - @property({reflect : true, type: {toAttribute}}) toAttribute = 1; + @property({converter : fromAttribute}) fromAttribute = 1; + @property({reflect : true, converter: {toAttribute}}) toAttribute = 1; @property({ attribute : 'all-attr', hasChanged, - type: {fromAttribute, toAttribute}, + converter: {fromAttribute, toAttribute}, reflect: true }) all = 10; @@ -450,14 +623,16 @@ suite('LitElement', () => { noAttr : {attribute : false}, atTr : {attribute : true}, customAttr : {attribute : 'custom', reflect : true}, - fromAttribute : {type : fromAttribute}, + fromAttribute : {converter : fromAttribute}, toAttribute : - {reflect : true, type : {toAttribute : toAttributeOnly}}, + {reflect : true, converter : {toAttribute : toAttributeOnly}}, all : { attribute : 'all-attr', - type : {fromAttribute, toAttribute}, + converter : {fromAttribute, toAttribute}, reflect : true }, + obj: {type: Object}, + arr: {type: Array} }; } @@ -467,6 +642,8 @@ suite('LitElement', () => { fromAttribute = 1; toAttribute: string|number = 1; all = 10; + obj?: any; + arr?: any; render() { return html``; } } @@ -478,7 +655,9 @@ suite('LitElement', () => { custom="3" fromAttribute="6-attr" toAttribute="7" - all-attr="11-attr">`; + all-attr="11-attr" + obj='{"foo": true, "bar": 5, "baz": "hi"}' + arr="[1, 2, 3, 4]">`; const el = container.firstChild as E; await el.updateComplete; assert.equal(el.noAttr, 'noAttr'); @@ -491,6 +670,8 @@ suite('LitElement', () => { assert.equal(el.getAttribute('toattribute'), '7-attr'); assert.equal(el.all, 11); assert.equal(el.getAttribute('all-attr'), '11-attr'); + assert.deepEqual(el.obj, {foo: true, bar: 5, baz: 'hi'}); + assert.deepEqual(el.arr, [1, 2, 3, 4]); }); if (Object.getOwnPropertySymbols) { @@ -540,7 +721,7 @@ suite('LitElement', () => { [zug] : { attribute : 'zug', reflect : true, - type : (value: string) => Number(value) + 100 + converter : (value: string) => Number(value) + 100 } }; } @@ -618,12 +799,12 @@ suite('LitElement', () => { class G extends F { static get properties(): PropertyDeclarations { return { - fromAttribute : {type : fromAttribute}, - toAttribute : {reflect : true, type : {toAttribute}}, + fromAttribute : {converter : fromAttribute}, + toAttribute : {reflect : true, converter : {toAttribute}}, all : { attribute : 'all-attr', hasChanged, - type : {fromAttribute, toAttribute}, + converter : {fromAttribute, toAttribute}, reflect : true }, }; @@ -746,7 +927,7 @@ suite('LitElement', () => { return { foo : { reflect : true, - type : {toAttribute : (value: any) => `${value}${suffix}`} + converter : {toAttribute : (value: any) => `${value}${suffix}`} } }; } @@ -919,7 +1100,7 @@ suite('LitElement', () => { bar : { attribute : 'attr-bar', reflect : true, - type : {fromAttribute, toAttribute}, + converter : {fromAttribute, toAttribute}, hasChanged } }; From c5c261e606b7e120814ed0705266cb32456346f3 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 13 Dec 2018 10:09:10 -0800 Subject: [PATCH 02/13] Format --- CHANGELOG.md | 1 - src/demo/ts-element.ts | 3 +- src/lib/decorators.ts | 4 +- src/lib/updating-element.ts | 40 ++++++++------- src/test/lit-element_test.ts | 95 ++++++++++++++++++------------------ 5 files changed, 73 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae7c513c..87a9d524 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed * 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`. In addition, numbers and strings now become null if their reflected attribute is removed. ([#264](https://github.com/Polymer/lit-element/issues/264)). - diff --git a/src/demo/ts-element.ts b/src/demo/ts-element.ts index b17e965e..3b1d7cbc 100644 --- a/src/demo/ts-element.ts +++ b/src/demo/ts-element.ts @@ -4,7 +4,8 @@ class TSElement extends LitElement { @property() message = 'Hi'; - @property({attribute : 'more-info', converter: (value: string) => `[${value}]`}) + @property( + {attribute : 'more-info', converter: (value: string) => `[${value}]`}) extra = ''; render() { diff --git a/src/lib/decorators.ts b/src/lib/decorators.ts index 58272bbe..d12b57ba 100644 --- a/src/lib/decorators.ts +++ b/src/lib/decorators.ts @@ -55,8 +55,8 @@ export const customElement = (tagName: string) => * corresponding attribute value. A `PropertyDeclaration` may optionally be * supplied to configure property features. */ -export const property = (options?: PropertyDeclaration) => (proto: Object, - name: PropertyKey) => { +export const property = (options?: PropertyDeclaration) => ( + proto: Object, name: PropertyKey) => { (proto.constructor as typeof UpdatingElement).createProperty(name, options); }; diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index e7afbc2e..aeac0a12 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -30,7 +30,8 @@ export interface ComplexAttributeConverter { toAttribute?(value: T, type?: any): string|null; } -type AttributeConverter = ComplexAttributeConverter|((value: string, type?: any) => T); +type AttributeConverter = + ComplexAttributeConverter|((value: string, type?: any) => T); /** * Defines options for a property accessor. @@ -101,24 +102,24 @@ export const defaultConverter: ComplexAttributeConverter = { toAttribute(value: any, type?: any) { switch (type) { - case Boolean: - return value ? '' : null; - case Object: - case Array: - return JSON.stringify(value); + case Boolean: + return value ? '' : null; + case Object: + case Array: + return 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); + case Boolean: + return value !== null; + case Number: + return value === null ? null : Number(value); + case Object: + case Array: + return JSON.parse(value); } return value; } @@ -141,7 +142,7 @@ export const notEqual: HasChanged = (value: unknown, old: unknown): boolean => { const defaultPropertyDeclaration: PropertyDeclaration = { attribute : true, type : String, - converter: defaultConverter, + converter : defaultConverter, reflect : false, hasChanged : notEqual }; @@ -292,14 +293,15 @@ export abstract class UpdatingElement extends HTMLElement { /** * Returns the property value for the given attribute value. - * Called via the `attributeChangedCallback` and uses the property's `converter` - * or `converter.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; const converter = options && options.converter || defaultConverter; - const fromAttribute = (typeof converter === 'function' ? converter : converter.fromAttribute); + const fromAttribute = + (typeof converter === 'function' ? converter : converter.fromAttribute); return fromAttribute ? fromAttribute(value, type) : value; } @@ -317,7 +319,9 @@ export abstract class UpdatingElement extends HTMLElement { } const type = options && options.type; const converter = options && options.converter; - const toAttribute = converter && (converter as ComplexAttributeConverter).toAttribute || defaultConverter.toAttribute; + const toAttribute = + converter && (converter as ComplexAttributeConverter).toAttribute || + defaultConverter.toAttribute; return toAttribute!(value, type); } diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index 12725f6a..d34145bb 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -246,23 +246,23 @@ suite('LitElement', () => { assert.equal(el.updateCount, 6); }); - test('property option `converter` can use `type` info', async() => { - const FooType = {name: 'FooType'}; + test('property option `converter` can use `type` info', async () => { + const FooType = {name : 'FooType'}; const converter = { - fromAttribute: (value: any, type: any) => { - return `fromAttribute: ${String(type.name)}`; - }, - toAttribute: (value: any, type: any) => { - return `toAttribute: ${String(type.name)}`; - } + fromAttribute : + (value: any, + type: any) => { return `fromAttribute: ${String(type.name)}`; }, + toAttribute : + (value: any, + type: any) => { return `toAttribute: ${String(type.name)}`; } }; class E extends LitElement { static get properties() { return { - num: {type: Number, converter, reflect: true}, - str: {type: String, converter, reflect: true}, - foo: {type: FooType, converter, reflect: true} + num : {type : Number, converter, reflect : true}, + str : {type : String, converter, reflect : true}, + foo : {type : FooType, converter, reflect : true} }; } @@ -299,26 +299,26 @@ suite('LitElement', () => { class E extends LitElement { static get properties() { return { - bool: {type: Boolean}, - num: {type: Number}, - str: {type: String}, - obj: {type: Object}, - arr: {type: Array}, - reflectBool: {type: Boolean, reflect: true}, - reflectNum: {type: Number, reflect: true}, - reflectStr: {type: String, reflect: true}, - reflectObj: {type: Object, reflect: true}, - reflectArr: {type: Array, reflect: true}, - defaultBool: {type: Boolean}, - defaultNum: {type: Number}, - defaultStr: {type: String}, - defaultObj: {type: Object}, - defaultArr: {type: Array}, - defaultReflectBool: {type: Boolean, reflect: true}, - defaultReflectNum: {type: Number, reflect: true}, - defaultReflectStr: {type: String, reflect: true}, - defaultReflectObj: {type: Object, reflect: true}, - defaultReflectArr: {type: Array, reflect: true}, + bool : {type : Boolean}, + num : {type : Number}, + str : {type : String}, + obj : {type : Object}, + arr : {type : Array}, + reflectBool : {type : Boolean, reflect : true}, + reflectNum : {type : Number, reflect : true}, + reflectStr : {type : String, reflect : true}, + reflectObj : {type : Object, reflect : true}, + reflectArr : {type : Array, reflect : true}, + defaultBool : {type : Boolean}, + defaultNum : {type : Number}, + defaultStr : {type : String}, + defaultObj : {type : Object}, + defaultArr : {type : Array}, + defaultReflectBool : {type : Boolean, reflect : true}, + defaultReflectNum : {type : Number, reflect : true}, + defaultReflectStr : {type : String, reflect : true}, + defaultReflectObj : {type : Object, reflect : true}, + defaultReflectArr : {type : Array, reflect : true}, }; } @@ -335,14 +335,13 @@ suite('LitElement', () => { defaultBool = false; defaultNum = 0; defaultStr = ''; - defaultObj = {defaultObj: false}; - defaultArr = [1]; + defaultObj = {defaultObj : false}; + defaultArr = [ 1 ]; defaultReflectBool = false; defaultReflectNum = 0; defaultReflectStr = 'defaultReflectStr'; - defaultReflectObj = {defaultReflectObj: true}; - defaultReflectArr = [1, 2]; - + defaultReflectObj = {defaultReflectObj : true}; + defaultReflectArr = [ 1, 2 ]; render() { return html``; } } @@ -359,23 +358,23 @@ suite('LitElement', () => { assert.equal(el.bool, true); assert.equal(el.num, 2); assert.equal(el.str, 'str'); - assert.deepEqual(el.obj, {obj: true}); - assert.deepEqual(el.arr, [1]); + assert.deepEqual(el.obj, {obj : true}); + assert.deepEqual(el.arr, [ 1 ]); assert.equal(el.reflectBool, true); assert.equal(el.reflectNum, 3); assert.equal(el.reflectStr, 'reflectStr'); - assert.deepEqual(el.reflectObj, {reflectObj: true}); - assert.deepEqual(el.reflectArr, [1, 2]); + assert.deepEqual(el.reflectObj, {reflectObj : true}); + assert.deepEqual(el.reflectArr, [ 1, 2 ]); assert.equal(el.defaultBool, true); assert.equal(el.defaultNum, 4); assert.equal(el.defaultStr, 'defaultStr'); - assert.deepEqual(el.defaultObj, {defaultObj: true}); - assert.deepEqual(el.defaultArr, [1, 2, 3]); + assert.deepEqual(el.defaultObj, {defaultObj : true}); + assert.deepEqual(el.defaultArr, [ 1, 2, 3 ]); assert.equal(el.defaultReflectBool, false); assert.equal(el.defaultReflectNum, 0); assert.equal(el.defaultReflectStr, 'defaultReflectStr'); - assert.deepEqual(el.defaultReflectObj, {defaultReflectObj: true}); - assert.deepEqual(el.defaultReflectArr, [1, 2]); + assert.deepEqual(el.defaultReflectObj, {defaultReflectObj : true}); + assert.deepEqual(el.defaultReflectArr, [ 1, 2 ]); el.removeAttribute('bool'); el.removeAttribute('num'); el.removeAttribute('str'); @@ -631,8 +630,8 @@ suite('LitElement', () => { converter : {fromAttribute, toAttribute}, reflect : true }, - obj: {type: Object}, - arr: {type: Array} + obj : {type : Object}, + arr : {type : Array} }; } @@ -670,8 +669,8 @@ suite('LitElement', () => { assert.equal(el.getAttribute('toattribute'), '7-attr'); assert.equal(el.all, 11); assert.equal(el.getAttribute('all-attr'), '11-attr'); - assert.deepEqual(el.obj, {foo: true, bar: 5, baz: 'hi'}); - assert.deepEqual(el.arr, [1, 2, 3, 4]); + assert.deepEqual(el.obj, {foo : true, bar : 5, baz : 'hi'}); + assert.deepEqual(el.arr, [ 1, 2, 3, 4 ]); }); if (Object.getOwnPropertySymbols) { From b384bdc7fea1021a43626b8417807a8fe8e7b97c Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 13 Dec 2018 17:55:40 -0800 Subject: [PATCH 03/13] Address review feedback --- CHANGELOG.md | 2 +- src/lib/updating-element.ts | 18 +++++++++--------- src/test/lit-element_test.ts | 9 +++++---- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64548450..9ebfceea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). * Types for the `property` and `customElement` decorators updated ([#288](https://github.com/Polymer/lit-element/issues/288) and [#291](https://github.com/Polymer/lit-element/issues/291)). ### Changed -* 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`. In addition, numbers and strings now become null if their reflected attribute is removed. ([#264](https://github.com/Polymer/lit-element/issues/264)). +* [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`. In addition, numbers and strings now become null if their reflected attribute is removed. ([#264](https://github.com/Polymer/lit-element/issues/264)). diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index aeac0a12..5aad2a57 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -15,28 +15,28 @@ /** * Converts property values to and from attribute values. */ -export interface ComplexAttributeConverter { +export interface ComplexAttributeConverter { /** * Deserializing function called to convert an attribute value to a property * value. */ - fromAttribute?(value: string, type?: any): T; + fromAttribute?(value: string, type?: TypeHint): Type; /** * Serializing function called to convert a property value to an attribute * value. */ - toAttribute?(value: T, type?: any): string|null; + toAttribute?(value: Type, type?: TypeHint): string|null; } -type AttributeConverter = - ComplexAttributeConverter|((value: string, type?: any) => T); +type AttributeConverter = + ComplexAttributeConverter|((value: string, type?: TypeHint) => Type); /** * Defines options for a property accessor. */ -export interface PropertyDeclaration { +export interface PropertyDeclaration { /** * Indicates how and whether the property becomes an observed attribute. @@ -52,7 +52,7 @@ export interface PropertyDeclaration { * converter to determine how to serialize and deserialize the attribute * to/from a property. */ - type?: T; + type?: TypeHint; /** * Indicates how to serialize and deserialize the attribute to/from a @@ -64,7 +64,7 @@ export interface PropertyDeclaration { * `reflect` is set to `true`, the property value is set directly to the * attribute. */ - converter?: AttributeConverter; + converter?: AttributeConverter; /** * Indicates if the property should reflect to an attribute. @@ -80,7 +80,7 @@ export interface PropertyDeclaration { * 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; } /** diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index d34145bb..0d0f90aa 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -17,7 +17,8 @@ import { LitElement, property, PropertyDeclarations, - PropertyValues + PropertyValues, + ComplexAttributeConverter } from '../lit-element.js'; import { @@ -248,12 +249,12 @@ suite('LitElement', () => { test('property option `converter` can use `type` info', async () => { const FooType = {name : 'FooType'}; - const converter = { + const converter: ComplexAttributeConverter = { fromAttribute : - (value: any, + (_value: any, type: any) => { return `fromAttribute: ${String(type.name)}`; }, toAttribute : - (value: any, + (_value: any, type: any) => { return `toAttribute: ${String(type.name)}`; } }; From abc945f73ca1b78ff6fb566fdf858f12015302be Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 13 Dec 2018 17:57:28 -0800 Subject: [PATCH 04/13] Update CHANGELOG.md --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ebfceea..daaa0475 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). Unreleased section, uncommenting the header as necessary. --> -## Unreleased -* Types for the `property` and `customElement` decorators updated ([#288](https://github.com/Polymer/lit-element/issues/288) and [#291](https://github.com/Polymer/lit-element/issues/291)). - ### 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`. In addition, numbers and strings now become null if their reflected attribute is removed. ([#264](https://github.com/Polymer/lit-element/issues/264)). From 3392e9bc97f97bcb5f9f47138cd5cf94c697401e Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 13 Dec 2018 18:00:09 -0800 Subject: [PATCH 05/13] Clarify documentation about default converter --- README.md | 2 ++ src/lib/updating-element.ts | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 070dd701..88f11990 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,8 @@ for additional information on how to create templates for lit-element. * `converter`: Indicates how to serialize and deserialize 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`. + 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 serialize and deserialize the attribute to/from a property. diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 5aad2a57..d4d92e89 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -62,7 +62,8 @@ export interface PropertyDeclaration { * deserialize function and `toAttribute` is a serialize function used to set * the property to an attribute. 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`. */ converter?: AttributeConverter; From f5f95e17238bafa35e87520d0836ea6ac378e2a7 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 13 Dec 2018 18:06:50 -0800 Subject: [PATCH 06/13] Format --- src/test/lit-element_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index 0d0f90aa..e3d8560a 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -13,12 +13,12 @@ */ import { + ComplexAttributeConverter, html, LitElement, property, PropertyDeclarations, - PropertyValues, - ComplexAttributeConverter + PropertyValues } from '../lit-element.js'; import { From 383510e31aa6eca33ccac699944f8daa0a47cd58 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 09:45:37 -0800 Subject: [PATCH 07/13] Fix test to use lowercase attribute names This is working around https://github.com/webcomponents/custom-elements/issues/167. --- src/test/lit-element_test.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index e3d8560a..b5387bc5 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -381,21 +381,21 @@ suite('LitElement', () => { el.removeAttribute('str'); el.removeAttribute('obj'); el.removeAttribute('arr'); - el.removeAttribute('reflectBool'); - el.removeAttribute('reflectNum'); - el.removeAttribute('reflectStr'); - el.removeAttribute('reflectObj'); - el.removeAttribute('reflectArr'); - el.removeAttribute('defaultBool'); - el.removeAttribute('defaultNum'); - el.removeAttribute('defaultStr'); - el.removeAttribute('defaultObj'); - el.removeAttribute('defaultArr'); - el.removeAttribute('defaultReflectBool'); - el.removeAttribute('defaultReflectNum'); - el.removeAttribute('defaultReflectStr'); - el.removeAttribute('defaultReflectObj'); - el.removeAttribute('defaultReflectArr'); + el.removeAttribute('reflectbool'); + el.removeAttribute('reflectnum'); + el.removeAttribute('reflectstr'); + el.removeAttribute('reflectobj'); + el.removeAttribute('reflectarr'); + el.removeAttribute('defaultbool'); + el.removeAttribute('defaultnum'); + el.removeAttribute('defaultstr'); + el.removeAttribute('defaultobj'); + el.removeAttribute('defaultarr'); + el.removeAttribute('defaultreflectbool'); + el.removeAttribute('defaultreflectnum'); + el.removeAttribute('defaultreflectstr'); + el.removeAttribute('defaultreflectobj'); + el.removeAttribute('defaultreflectarr'); await el.updateComplete; assert.equal(el.bool, false); assert.equal(el.num, null); From 0ba8f1be1e74113032a43c80f971f5f80bc54c2d Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 10:06:49 -0800 Subject: [PATCH 08/13] Fix test on IE --- src/test/lit-element_test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index b5387bc5..4c8a178b 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -249,6 +249,14 @@ suite('LitElement', () => { test('property option `converter` can use `type` info', async () => { const FooType = {name : 'FooType'}; + // Make test work on IE where these are undefined. + if (!('name' in String)) { + (String as any).name = (String as any).name || 'String'; + } + if (!('name' in Number)) { + (Number as any).name = (Number as any).name || 'Number'; + } + const converter: ComplexAttributeConverter = { fromAttribute : (_value: any, From 6408fc50d69a6d85f0923cfee2a3ce066bbd8437 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 10:24:04 -0800 Subject: [PATCH 09/13] Address review feedback Remove superfluous null checks --- src/lib/updating-element.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index d4d92e89..346790cf 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -272,8 +272,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' @@ -298,9 +298,9 @@ export abstract class UpdatingElement extends HTMLElement { * `converter` or `converter.fromAttribute` property option. */ private static _propertyValueFromAttribute(value: string, - options?: PropertyDeclaration) { - const type = options && options.type; - const converter = options && options.converter || defaultConverter; + options: PropertyDeclaration) { + const type = options.type; + const converter = options.converter || defaultConverter; const fromAttribute = (typeof converter === 'function' ? converter : converter.fromAttribute); return fromAttribute ? fromAttribute(value, type) : value; @@ -314,12 +314,12 @@ 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; } - const type = options && options.type; - const converter = options && options.converter; + const type = options.type; + const converter = options.converter; const toAttribute = converter && (converter as ComplexAttributeConverter).toAttribute || defaultConverter.toAttribute; @@ -472,7 +472,7 @@ export abstract class UpdatingElement extends HTMLElement { const ctor = (this.constructor as typeof UpdatingElement); const propName = ctor._attributeToPropertyMap.get(name); if (propName !== undefined) { - const options = ctor._classProperties.get(propName); + const options = ctor._classProperties.get(propName) || defaultPropertyDeclaration; this[propName as keyof this] = ctor._propertyValueFromAttribute(value, options); } From 7bee8e42ccb8959509b5091158956eaba26cea25 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 14:54:11 -0800 Subject: [PATCH 10/13] Makes reflection more consistent 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 This change helps ensure that when a user calls `setAttribute`, a `converter.toAttribute` does not cause the attribute to immediately mutate. This is unexpected behavior and this change discourages it. --- CHANGELOG.md | 5 ++++- README.md | 4 +++- src/lib/updating-element.ts | 33 +++++++++++++++++++++------------ src/test/lit-element_test.ts | 13 ++++++++++--- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index daaa0475..2dba416e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). --> ### 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`. In addition, numbers and strings now become null if their reflected attribute is removed. ([#264](https://github.com/Polymer/lit-element/issues/264)). +* [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)) diff --git a/README.md b/README.md index 88f11990..379191d6 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,9 @@ for additional information on how to create templates for lit-element. `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 serialize and deserialize the attribute - to/from a property. + 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 visa 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 diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 346790cf..9b6d195d 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -63,7 +63,10 @@ export interface PropertyDeclaration { * the property to an attribute. If no `toAttribute` function is provided and * `reflect` is set to `true`, the property value is set directly to the * attribute. A default `converter` is used if none is provided; it supports - * `Boolean`, `String`, `Number`, `Object`, and `Array`. + * `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 visa versa. */ converter?: AttributeConverter; @@ -468,14 +471,19 @@ export abstract class UpdatingElement extends HTMLElement { 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) || defaultPropertyDeclaration; - this[propName as keyof this] = - ctor._propertyValueFromAttribute(value, options); - } + if (this._updateState & STATE_IS_REFLECTING) { + 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; + this[propName as keyof this] = + ctor._propertyValueFromAttribute(value, options); + // mark state not reflecting + this._updateState = this._updateState & ~STATE_IS_REFLECTING; } } @@ -515,12 +523,13 @@ export abstract class UpdatingElement extends HTMLElement { options.hasChanged)) { return this.updateComplete; } - // track old value when changing. + // Track old value when changing. if (!this._changedProperties.has(name)) { 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 from the attribute + if (options.reflect === true && !(this._updateState & STATE_IS_REFLECTING)) { if (this._reflectingProperties === undefined) { this._reflectingProperties = new Map(); } diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index 4c8a178b..0dfc8b68 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -299,6 +299,13 @@ suite('LitElement', () => { assert.equal(el.num, 'fromAttribute: Number'); assert.equal(el.str, 'fromAttribute: String'); assert.equal(el.foo, 'fromAttribute: FooType'); + assert.equal(el.getAttribute('num'), null); + assert.equal(el.getAttribute('str'), null); + assert.equal(el.getAttribute('foo'), null); + el.num = 0; + el.str = ''; + el.foo = {}; + await el.updateComplete; assert.equal(el.getAttribute('num'), 'toAttribute: Number'); assert.equal(el.getAttribute('str'), 'toAttribute: String'); assert.equal(el.getAttribute('foo'), 'toAttribute: FooType'); @@ -753,7 +760,7 @@ suite('LitElement', () => { assert.equal(el.getAttribute('zug'), '6'); el.setAttribute('zug', '7'); await el.updateComplete; - assert.equal(el.getAttribute('zug'), '107'); + assert.equal(el.getAttribute('zug'), '7'); assert.equal(el[zug], 107); }); } @@ -1145,12 +1152,12 @@ suite('LitElement', () => { await el.updateComplete; assert.equal(el.updateCount, 2); assert.equal(el.bar, 7); - assert.equal(el.getAttribute('attr-bar'), `7-attr`); + assert.equal(el.getAttribute('attr-bar'), `7`); el.bar = 4; await el.updateComplete; assert.equal(el.updateCount, 2); assert.equal(el.bar, 4); - assert.equal(el.getAttribute('attr-bar'), `7-attr`); + assert.equal(el.getAttribute('attr-bar'), `7`); el.setAttribute('attr-bar', '3'); await el.updateComplete; assert.equal(el.updateCount, 2); From f2748f33cd17e509c24e720a6a7c28093b12ebf2 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 16:28:13 -0800 Subject: [PATCH 11/13] Format. --- src/lib/updating-element.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 9b6d195d..4694b6ea 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -477,7 +477,8 @@ export abstract class UpdatingElement extends HTMLElement { const ctor = (this.constructor as typeof UpdatingElement); const propName = ctor._attributeToPropertyMap.get(name); if (propName !== undefined) { - const options = ctor._classProperties.get(propName) || defaultPropertyDeclaration; + const options = + ctor._classProperties.get(propName) || defaultPropertyDeclaration; // mark state reflecting this._updateState = this._updateState | STATE_IS_REFLECTING; this[propName as keyof this] = @@ -529,7 +530,8 @@ export abstract class UpdatingElement extends HTMLElement { } // Add to reflecting properties set if `reflect` is true and the property // is not reflecting from the attribute - if (options.reflect === true && !(this._updateState & STATE_IS_REFLECTING)) { + if (options.reflect === true && + !(this._updateState & STATE_IS_REFLECTING)) { if (this._reflectingProperties === undefined) { this._reflectingProperties = new Map(); } From 168c38438a218b64ac808860feacd48f2e7cbd54 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 17:35:25 -0800 Subject: [PATCH 12/13] Address review feedback. --- README.md | 18 +++++++-------- src/lib/updating-element.ts | 43 +++++++++++++++++++----------------- src/test/lit-element_test.ts | 30 +++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 379191d6..7df56077 100644 --- a/README.md +++ b/README.md @@ -35,24 +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'`). - * `converter`: 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`. 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 serialize and deserialize the attribute + `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 visa versa. + 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. diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 4694b6ea..6e494df1 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -18,13 +18,13 @@ export interface ComplexAttributeConverter { /** - * 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, 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: Type, type?: TypeHint): string|null; @@ -49,24 +49,22 @@ export interface PropertyDeclaration { /** * Indicates the type of the property. This is used only as a hint for the - * converter to determine how to serialize and deserialize the attribute + * `converter` to determine how to convert the attribute * to/from a property. */ type?: TypeHint; /** - * 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 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. 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 visa versa. + * and vice versa. */ converter?: AttributeConverter; @@ -74,7 +72,7 @@ export interface PropertyDeclaration { * 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 + * property option and the value of the property converted using the rules * from the `converter` property option. */ reflect?: boolean; @@ -155,9 +153,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 @@ -449,6 +448,10 @@ export abstract class UpdatingElement extends HTMLElement { 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. @@ -457,21 +460,21 @@ export abstract class UpdatingElement extends HTMLElement { // 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; + 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; + 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) { + if (this._updateState & STATE_IS_REFLECTING_TO_ATTRIBUTE) { return; } const ctor = (this.constructor as typeof UpdatingElement); @@ -480,11 +483,11 @@ export abstract class UpdatingElement extends HTMLElement { const options = ctor._classProperties.get(propName) || defaultPropertyDeclaration; // mark state reflecting - this._updateState = this._updateState | STATE_IS_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; + this._updateState = this._updateState & ~STATE_IS_REFLECTING_TO_PROPERTY; } } @@ -529,9 +532,9 @@ export abstract class UpdatingElement extends HTMLElement { this._changedProperties.set(name, oldValue); } // Add to reflecting properties set if `reflect` is true and the property - // is not reflecting from the attribute + // is not reflecting to the property from the attribute if (options.reflect === true && - !(this._updateState & STATE_IS_REFLECTING)) { + !(this._updateState & STATE_IS_REFLECTING_TO_PROPERTY)) { if (this._reflectingProperties === undefined) { this._reflectingProperties = new Map(); } diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index 0dfc8b68..10e085e2 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -434,6 +434,36 @@ suite('LitElement', () => { assert.deepEqual(el.defaultReflectArr, null); }); + test('if a `reflect: true` property\'s `converter.toAttribute` returns `undefined`, the attribute does not change', async () => { + class E extends LitElement { + static get properties() { + return { + foo: {reflect: true} + }; + } + + foo?: any; + render() { return html``; } + } + const name = generateElementName(); + customElements.define(name, E); + const el = new E(); + container.appendChild(el); + await el.updateComplete; + el.setAttribute('foo', 'foo'); + assert.equal(el.foo, 'foo'); + await el.updateComplete; + el.foo = 'foo2'; + await el.updateComplete; + assert.equal(el.getAttribute('foo'), 'foo2'); + el.foo = undefined; + await el.updateComplete; + assert.equal(el.getAttribute('foo'), 'foo2'); + el.foo = 'foo3'; + await el.updateComplete; + assert.equal(el.getAttribute('foo'), 'foo3'); + }); + test('property options via decorator', async () => { const hasChanged = (value: any, old: any) => old === undefined || value > old; From 54f060ffc7ed8d532568f7d4422e5969145ff62a Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Fri, 14 Dec 2018 18:11:13 -0800 Subject: [PATCH 13/13] Address review feedback Ensure Object/Array properties respect `undefined` (no change to attribute) and `null` (remove attribute) values. --- src/lib/updating-element.ts | 4 ++- src/test/lit-element_test.ts | 55 ++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 6e494df1..a6da2c59 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -108,7 +108,9 @@ export const defaultConverter: ComplexAttributeConverter = { return value ? '' : null; case Object: case Array: - return JSON.stringify(value); + // 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; }, diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index 10e085e2..7833ec8f 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -434,15 +434,58 @@ suite('LitElement', () => { assert.deepEqual(el.defaultReflectArr, null); }); - test('if a `reflect: true` property\'s `converter.toAttribute` returns `undefined`, the attribute does not change', async () => { + test('attributes removed when a reflecting property\'s value becomes null', async () => { class E extends LitElement { static get properties() { return { - foo: {reflect: true} + bool : {type : Boolean, reflect: true}, + num : {type : Number, reflect: true}, + str : {type : String, reflect: true}, + obj : {type : Object, reflect: true}, + arr : {type : Array, reflect: true} + }; + } + + bool?: any; + num?: any; + str?: any; + obj?: any; + arr?: any; + + render() { return html``; } + } + const name = generateElementName(); + customElements.define(name, E); + container.innerHTML = `<${name} bool num="2" str="str" obj='{"obj": true}' + arr='[1]'> + `; + const el = container.firstChild as E; + await el.updateComplete; + el.bool = false; + el.num = null; + el.str = null; + el.obj = null; + el.arr = null; + await el.updateComplete; + assert.isFalse(el.hasAttribute('bool')); + assert.isFalse(el.hasAttribute('num')); + assert.isFalse(el.hasAttribute('str')); + assert.isFalse(el.hasAttribute('obj')); + assert.isFalse(el.hasAttribute('arr')); + }); + + test('if a `reflect: true` returns `undefined`, the attribute does not change', async () => { + class E extends LitElement { + static get properties() { + return { + foo: {reflect: true}, + obj: {type: Object, reflect: true} }; } foo?: any; + obj?: any; + render() { return html``; } } const name = generateElementName(); @@ -451,17 +494,25 @@ suite('LitElement', () => { container.appendChild(el); await el.updateComplete; el.setAttribute('foo', 'foo'); + el.setAttribute('obj', '{"obj": 1}'); assert.equal(el.foo, 'foo'); + assert.deepEqual(el.obj, {obj: 1}); await el.updateComplete; el.foo = 'foo2'; + el.obj = {obj: 2}; await el.updateComplete; assert.equal(el.getAttribute('foo'), 'foo2'); + assert.equal(el.getAttribute('obj'), '{"obj":2}'); el.foo = undefined; + el.obj = undefined; await el.updateComplete; assert.equal(el.getAttribute('foo'), 'foo2'); + assert.equal(el.getAttribute('obj'), '{"obj":2}'); el.foo = 'foo3'; + el.obj = {obj: 3}; await el.updateComplete; assert.equal(el.getAttribute('foo'), 'foo3'); + assert.equal(el.getAttribute('obj'), '{"obj":3}'); }); test('property options via decorator', async () => {