Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes createProperty easier to use to customize properties #914

Merged
merged 16 commits into from
Mar 10, 2020
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## Unreleased

### Changed
* The value returned by `render` is always rendered, even if it isn't a `TemplateResult`. ([#712](https://github.com/Polymer/lit-element/issues/712)
* Added a static `createPropertyDescriptor` method to allow easier customization of property accessors. This method should return a a `PropertyDescriptor` to install on the property. If no descriptor is returned, a property accessor is not be created. ([#911](https://github.com/Polymer/lit-element/issues/911))
* The value returned by `render` is always rendered, even if it isn't a `TemplateResult`. ([#712](https://github.com/Polymer/lit-element/issues/712))

### Added
* Added `@queryAsync(selector)` decorator which returns a Promise that resolves to the result of querying for the given selector after the element's `updateComplete` Promise resolves ([#903](https://github.com/Polymer/lit-element/issues/903)).
Expand Down
2 changes: 1 addition & 1 deletion src/demo/ts-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class TSElement extends LitElement {
@property() message = 'Hi';

@property(
{attribute: 'more-info', converter: (value: string) => `[${value}]`})
{attribute: 'more-info', converter: (value: string|null) => `[${value}]`})
extra = '';

render() {
Expand Down
73 changes: 59 additions & 14 deletions src/lib/updating-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface ComplexAttributeConverter<Type = unknown, TypeHint = unknown> {
}

type AttributeConverter<Type = unknown, TypeHint = unknown> =
ComplexAttributeConverter<Type>|((value: string, type?: TypeHint) => Type);
ComplexAttributeConverter<Type>|((value: string|null, type?: TypeHint) => Type);

/**
* Defines options for a property accessor.
Expand Down Expand Up @@ -112,6 +112,10 @@ export interface PropertyDeclaration<Type = unknown, TypeHint = unknown> {
* the property changes.
*/
readonly noAccessor?: boolean;

// Allows extension while preserving the ability to use the
// @property decorator.
[index: string]: unknown;
}

/**
Expand Down Expand Up @@ -304,7 +308,38 @@ export abstract class UpdatingElement extends HTMLElement {
return;
}
const key = typeof name === 'symbol' ? Symbol() : `__${name}`;
sorvell marked this conversation as resolved.
Show resolved Hide resolved
Object.defineProperty(this.prototype, name, {
const descriptor = this.createPropertyDescriptor(name, key, options);
if (descriptor !== undefined) {
Object.defineProperty(this.prototype, name, descriptor);
}
}

/**
* Creates a property descriptor to be defined on the given named property.
* If no descriptor is returned, the property will not become an accessor.
* For example,
*
* class MyElement extends LitElement {
* static createPropertyDescriptor(name, key, options) {
* const defaultDescriptor = super.createPropertyDescriptor(name, key, options);
* const setter = defaultDescriptor.set;
* return {
* get: defaultDescriptor.get,
* set(value) {
* setter.call(this, value);
* // custom action.
* },
* configurable: true,
* enumerable: true
* }
* }
* }
*
* @nocollapse
*/
protected static createPropertyDescriptor(name: PropertyKey,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we name this get...? On getX vs. createX it seems like the distinction is that createX is side effectful and getX isn't.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

key: string|symbol, _options: PropertyDeclaration,) {
return {
// tslint:disable-next-line:no-any no symbol in index
get(): any {
return (this as {[key: string]: unknown})[key as string];
Expand All @@ -317,7 +352,12 @@ export abstract class UpdatingElement extends HTMLElement {
},
configurable: true,
enumerable: true
});
};
}

protected static getDeclarationForProperty(name: PropertyKey) {
sorvell marked this conversation as resolved.
Show resolved Hide resolved
return this._classProperties && this._classProperties.get(name) ||
defaultPropertyDeclaration;
}

/**
Expand Down Expand Up @@ -392,9 +432,10 @@ export abstract class UpdatingElement extends HTMLElement {
private static _propertyValueFromAttribute(
value: string|null, options: PropertyDeclaration) {
const type = options.type;
const converter = options.converter || defaultConverter;
const converter = options.converter ||
defaultPropertyDeclaration.converter;
const fromAttribute =
(typeof converter === 'function' ? converter : converter.fromAttribute);
(typeof converter === 'function' ? converter : converter!.fromAttribute);
return fromAttribute ? fromAttribute(value, type) : value;
}

Expand All @@ -412,11 +453,14 @@ export abstract class UpdatingElement extends HTMLElement {
return;
}
const type = options.type;
const converter = options.converter;
const toAttribute =
converter && (converter as ComplexAttributeConverter).toAttribute ||
defaultConverter.toAttribute;
return toAttribute!(value, type);
let converter = options.converter;
let toAttribute =
converter && (converter as ComplexAttributeConverter).toAttribute;
if (!toAttribute) {
converter = defaultPropertyDeclaration.converter;
toAttribute = converter && (converter as ComplexAttributeConverter).toAttribute;
}
return toAttribute ? toAttribute(value, type) : value;
sorvell marked this conversation as resolved.
Show resolved Hide resolved
}

private _updateState: UpdateState = 0;
Expand Down Expand Up @@ -563,8 +607,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) || defaultPropertyDeclaration;
const options = ctor.getDeclarationForProperty(propName);
// mark state reflecting
this._updateState = this._updateState | STATE_IS_REFLECTING_TO_PROPERTY;
this[propName as keyof this] =
Expand All @@ -585,8 +628,7 @@ export abstract class UpdatingElement extends HTMLElement {
// If we have a property key, perform property update steps.
if (name !== undefined) {
const ctor = this.constructor as typeof UpdatingElement;
const options =
ctor._classProperties!.get(name) || defaultPropertyDeclaration;
const options = ctor.getDeclarationForProperty(name);
if (ctor._valueHasChanged(
this[name as keyof this], oldValue, options.hasChanged)) {
if (!this._changedProperties.has(name)) {
Expand Down Expand Up @@ -679,6 +721,9 @@ export abstract class UpdatingElement extends HTMLElement {
* ```
*/
protected performUpdate(): void|Promise<unknown> {
if (!this._hasRequestedUpdate) {
return;
sorvell marked this conversation as resolved.
Show resolved Hide resolved
}
// Mixin instance properties once, if they exist.
if (this._instanceProperties) {
this._applyInstanceProperties();
Expand Down
134 changes: 131 additions & 3 deletions src/test/lib/updating-element_test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/**
/**
* @license
* Copyright (c) 2018 The Polymer Project Authors. All rights reserved.
* This code may only be used under the BSD style license found at
Expand All @@ -12,8 +12,8 @@
* http://polymer.github.io/PATENTS.txt
*/

import {property} from '../../lib/decorators.js';
import {ComplexAttributeConverter, PropertyDeclarations, PropertyValues, UpdatingElement} from '../../lib/updating-element.js';
import {property, customElement} from '../../lib/decorators.js';
import {ComplexAttributeConverter, PropertyDeclarations, PropertyValues, UpdatingElement, PropertyDeclaration, defaultConverter} from '../../lib/updating-element.js';
import {generateElementName} from '../test-helpers.js';

// tslint:disable:no-any ok in tests
Expand Down Expand Up @@ -1802,6 +1802,134 @@ suite('UpdatingElement', () => {
assert.equal(sub.getAttribute('foo'), '5');
});

test('can provide a default property declaration', async () => {
sorvell marked this conversation as resolved.
Show resolved Hide resolved

const SpecialNumber = {};

const myPropertyDeclaration = {
type: SpecialNumber,
reflect: true,
converter: {
toAttribute: function(value: unknown, type?: unknown): unknown {
switch (type) {
case String:
return value === undefined ? null : value;
default:
return defaultConverter.toAttribute!(value, type);
}
},
fromAttribute: function(value: string|null, type?: unknown) {
switch (type) {
case SpecialNumber:
return Number(value) + 10;
default:
return defaultConverter.fromAttribute!(value, type);
}
}
}
};

@customElement(generateElementName())
class E extends UpdatingElement {

static createProperty(
name: PropertyKey,
options: PropertyDeclaration) {
// Always mix into defaults to preserve custom converter.
options = Object.assign(Object.create(myPropertyDeclaration), options);
super.createProperty(name, options);
}

@property()
foo = 5;

@property({type: String})
bar?: string = 'bar';
}

const el = new E();
container.appendChild(el);
el.setAttribute('foo', '10');
el.setAttribute('bar', 'attrBar');
await el.updateComplete;
assert.equal(el.foo, 20);
assert.equal(el.bar, 'attrBar');
el.foo = 5;
el.bar = undefined;
await el.updateComplete;
assert.equal(el.getAttribute('foo'), '5');
assert.isFalse(el.hasAttribute('bar'));
});

test('can customize property options and accessor creation', async () => {

interface MyPropertyDeclaration<TypeHint = unknown> extends PropertyDeclaration {
validator?: (value: any) => TypeHint;
observer?: (oldValue: TypeHint) => void;
}

@customElement(generateElementName())
class E extends UpdatingElement {

static createPropertyDescriptor(name: PropertyKey, key: string|symbol, options: MyPropertyDeclaration) {
const defaultDescriptor = super.createPropertyDescriptor(name, key, options);
return {
get: defaultDescriptor!.get,
set(this: E, value: unknown) {
const oldValue =
(this as unknown as {[key: string]: unknown})[name as string];
if (options.validator) {
value = options.validator(value);
}
(this as unknown as {[key: string]: unknown})[key as string] = value;
(this as unknown as UpdatingElement).requestUpdate(name, oldValue);
},

configurable: defaultDescriptor.configurable,
enumerable: defaultDescriptor.enumerable
};
}

updated(changedProperties: PropertyValues) {
super.updated(changedProperties);
changedProperties.forEach((value: unknown, key: PropertyKey) => {
const options = (this.constructor as typeof UpdatingElement)
.getDeclarationForProperty(key) as MyPropertyDeclaration;
const observer = options.observer;
if (typeof observer === 'function') {
observer.call(this, value);
}
});
}

@property({type: Number, validator: (value: number) => Math.min(10, Math.max(value, 0))})
foo = 5;

@property({})
bar = 'bar';

// tslint:disable-next-line:no-any
_observedZot?: any;

@property({observer: function(this: E, oldValue: string) { this._observedZot = {value: this.zot, oldValue}; } })
zot = '';
}

const el = new E();
container.appendChild(el);
await el.updateComplete;
el.foo = 20;
assert.equal(el.foo, 10);
assert.deepEqual(el._observedZot, {value: '', oldValue: undefined});
el.foo = -5;
assert.equal(el.foo, 0);
el.bar = 'bar2';
assert.equal(el.bar, 'bar2');
el.zot = 'zot';
await el.updateComplete;
assert.deepEqual(el._observedZot, {value: 'zot', oldValue: ''});
});

test('attribute-based property storage', async () => {
class E extends UpdatingElement {
_updateCount = 0;
Expand Down