Skip to content

Commit

Permalink
Changes property options to support converter (#369)
Browse files Browse the repository at this point in the history 
* 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.

* Format

* Address review feedback

* Update CHANGELOG.md

* Clarify documentation about default converter

* Format

* Fix test to use lowercase attribute names

This is working around https://github.com/webcomponents/custom-elements/issues/167.

* Fix test on IE

* Address review feedback

Remove superfluous null checks

* 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.

* Format.

* Address review feedback.

* Address review feedback

Ensure Object/Array properties respect `undefined` (no change to attribute) and `null` (remove attribute) values.
  • Loading branch information
@kevinpschaaf
Steve Orvell authored and kevinpschaaf committed Dec 17, 2018
1 parent 28d4b3c commit 47717a7
Show file tree
Hide file tree
Showing 6 changed files with 442 additions and 119 deletions.
19 changes: 10 additions & 9 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 -->
23 changes: 13 additions & 10 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion demo/lit-element.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/demo/ts-element.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
193 changes: 117 additions & 76 deletions src/lib/updating-element.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
};
Expand All @@ -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
Expand Down Expand Up @@ -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'
Expand All @@ -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;
}

/**
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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();
}
Expand Down
Loading

0 comments on commit 47717a7

Please sign in to comment.